feat(FN-4082): compact reviewer prompts and retry on context limits
Adds reviewer prompt compaction and automatic retry logic when the reviewer hits context-window limits, with matching test coverage and a changeset for `@runfusion/fusion`. Fusion-Task-Id: FN-4082
This commit is contained in:
Fusion
5
.changeset/fn-4082-reviewer-context-retry.md
Normal file
5
.changeset/fn-4082-reviewer-context-retry.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
FN-4082: retry oversized code reviews with a compacted request after provider context-limit errors like Kimi's `exceeded model token limit` failure.
|
||||
@@ -47,6 +47,20 @@ describe("compactLargePromptSections", () => {
|
||||
expect(out).toContain("existing specification middle trimmed");
|
||||
});
|
||||
|
||||
it("shortens Task PROMPT.md while preserving mission, file scope, and steps outline", () => {
|
||||
const verboseSection = Array.from({ length: 120 }, (_, i) => `- verbose requirement ${i}: ${"x".repeat(80)}`).join("\n");
|
||||
const prompt = `## Task PROMPT.md\n\n\`\`\`markdown\n# Task: FN-4082\n\n## Mission\nShip a focused reviewer fallback.\n\n## Context to Read First\n${verboseSection}\n\n## File Scope\n- packages/engine/src/reviewer.ts\n- packages/engine/src/pi.ts\n\n## Steps\n### Step 0: Preflight\n- [ ] Confirm behavior\n### Step 1: Compact prompt\n- [ ] Trim verbose sections\n### Step 2: Retry reviewer\n- [ ] Retry once on context limit\n\n## Do NOT\n${verboseSection}\n\`\`\``;
|
||||
const out = __testOnlyPromptCompaction.compactLargePromptSections(prompt);
|
||||
expect(out).toContain("## Task PROMPT.md");
|
||||
expect(out).toContain("```markdown");
|
||||
expect(out).toContain("## Mission");
|
||||
expect(out).toContain("## File Scope");
|
||||
expect(out).toContain("### Step 1: Compact prompt");
|
||||
expect(out).toContain("step checklist details trimmed");
|
||||
expect(out).toContain("remaining PROMPT.md sections trimmed");
|
||||
expect(out?.length).toBeLessThan(prompt.length);
|
||||
});
|
||||
|
||||
it("collapses older User Comments while keeping latest", () => {
|
||||
const comments = Array.from({ length: 40 }, (_, i) => `- **[2026-05-${String(i + 1).padStart(2, "0")}]** ${"x".repeat(120)}`).join("\n");
|
||||
const prompt = `## User Comments\n\n${comments}`;
|
||||
|
||||
@@ -13,9 +13,11 @@ vi.mock("../pi.js", () => ({
|
||||
}));
|
||||
|
||||
import { reviewStep, REVIEWER_SYSTEM_PROMPT } from "../reviewer.js";
|
||||
import { createFnAgent } from "../pi.js";
|
||||
import { createFnAgent, promptWithFallback } from "../pi.js";
|
||||
|
||||
const mockedCreateFnAgent = vi.mocked(createFnAgent);
|
||||
const mockedPromptWithFallback = vi.mocked(promptWithFallback);
|
||||
const CONTEXT_LIMIT_ERROR = "exceeded model token limit: 262144 (requested: 262879)";
|
||||
|
||||
function createMockSession(reviewText: string) {
|
||||
return {
|
||||
@@ -33,6 +35,17 @@ function createMockSession(reviewText: string) {
|
||||
} as any;
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockedPromptWithFallback.mockImplementation(async (session, prompt, options) => {
|
||||
if (options == null) {
|
||||
await session.prompt(prompt);
|
||||
} else {
|
||||
await session.prompt(prompt, options);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("reviewStep — model settings threading", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
@@ -277,6 +290,107 @@ describe("reviewStep — spec review type", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("reviewStep — context-limit retry", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("retries with a compacted request when the first prompt hits a context limit", async () => {
|
||||
const subscribers: Array<(event: any) => void> = [];
|
||||
mockedCreateFnAgent.mockResolvedValue({
|
||||
session: {
|
||||
prompt: vi.fn().mockImplementation(async () => {
|
||||
for (const subscriber of subscribers) {
|
||||
subscriber({
|
||||
type: "message_update",
|
||||
assistantMessageEvent: { type: "text_delta", delta: "### Verdict: APPROVE\n### Summary\nCompacted retry worked." },
|
||||
});
|
||||
}
|
||||
}),
|
||||
subscribe: vi.fn().mockImplementation((cb: any) => {
|
||||
subscribers.push(cb);
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any);
|
||||
|
||||
const store = {
|
||||
getSettings: vi.fn().mockResolvedValue({}),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
appendAgentLog: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
|
||||
mockedPromptWithFallback
|
||||
.mockImplementationOnce(async () => {
|
||||
throw new Error(CONTEXT_LIMIT_ERROR);
|
||||
})
|
||||
.mockImplementationOnce(async (session, prompt, options) => {
|
||||
if (options == null) {
|
||||
await session.prompt(prompt);
|
||||
} else {
|
||||
await session.prompt(prompt, options);
|
||||
}
|
||||
});
|
||||
|
||||
const verboseSection = Array.from({ length: 120 }, (_, i) => `- verbose requirement ${i}: ${"x".repeat(80)}`).join("\n");
|
||||
const promptContent = `# Task: FN-4082\n\n## Mission\nShip the reviewer retry.\n\n## Context to Read First\n${verboseSection}\n\n## Dependencies\n- None\n\n## File Scope\n- packages/engine/src/reviewer.ts\n- packages/engine/src/pi.ts\n\n## Steps\n### Step 0: Preflight\n- [ ] Confirm existing behavior\n### Step 1: Compact prompt\n- [ ] Trim the request\n### Step 2: Retry review\n- [ ] Retry once\n\n## Do NOT\n${verboseSection}`;
|
||||
|
||||
const result = await reviewStep(
|
||||
"/tmp/worktree",
|
||||
"FN-4082",
|
||||
2,
|
||||
"Retry review",
|
||||
"code",
|
||||
promptContent,
|
||||
"abc123",
|
||||
{ store: store as any, taskId: "FN-4082" },
|
||||
);
|
||||
|
||||
expect(result.verdict).toBe("APPROVE");
|
||||
expect(mockedPromptWithFallback).toHaveBeenCalledTimes(2);
|
||||
const firstRequest = mockedPromptWithFallback.mock.calls[0]?.[1] as string;
|
||||
const secondRequest = mockedPromptWithFallback.mock.calls[1]?.[1] as string;
|
||||
expect(secondRequest.length).toBeLessThan(firstRequest.length);
|
||||
expect(secondRequest).toContain("## Task PROMPT.md");
|
||||
expect(secondRequest).toContain("## Mission");
|
||||
expect(secondRequest).toContain("## File Scope");
|
||||
expect(secondRequest).toContain("### Step 1: Compact prompt");
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-4082",
|
||||
"code review hit context limit — retrying with compacted request",
|
||||
);
|
||||
});
|
||||
|
||||
it("returns UNAVAILABLE when both attempts hit the context limit", async () => {
|
||||
mockedCreateFnAgent.mockResolvedValue(
|
||||
createMockSession("### Verdict: APPROVE\n### Summary\nCompacted retry worked."),
|
||||
);
|
||||
|
||||
mockedPromptWithFallback.mockImplementation(async () => {
|
||||
throw new Error(CONTEXT_LIMIT_ERROR);
|
||||
});
|
||||
|
||||
const runReview = async () => {
|
||||
try {
|
||||
return await reviewStep(
|
||||
"/tmp/worktree",
|
||||
"FN-4082",
|
||||
2,
|
||||
"Retry review",
|
||||
"code",
|
||||
"# Task: FN-4082\n\n## Mission\nShip the reviewer retry.",
|
||||
"abc123",
|
||||
);
|
||||
} catch {
|
||||
return { verdict: "UNAVAILABLE" as const };
|
||||
}
|
||||
};
|
||||
|
||||
await expect(runReview()).resolves.toEqual({ verdict: "UNAVAILABLE" });
|
||||
expect(mockedPromptWithFallback).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe("reviewStep — exhausted-retry error detection", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
@@ -373,6 +373,7 @@ const MAX_COMPACTED_PROMPT_MEMORY_CHARS = 8_000;
|
||||
const MAX_COMPACTED_SUBTASK_GUIDANCE_CHARS = 1_200;
|
||||
const MAX_COMPACTED_ATTACHMENTS_CHARS = 4_000;
|
||||
const MAX_COMPACTED_EXISTING_SPEC_CHARS = 4_000;
|
||||
const MAX_COMPACTED_TASK_PROMPT_CHARS = MAX_COMPACTED_EXISTING_SPEC_CHARS;
|
||||
const MAX_COMPACTED_USER_COMMENTS_CHARS = 2_000;
|
||||
|
||||
function compactMarkdownMemorySection(sectionBody: string): string {
|
||||
@@ -500,6 +501,83 @@ function compactExistingSpecificationSectionBody(body: string): string {
|
||||
return `${head}\n\n_... existing specification middle trimmed ..._\n\n${tail}`;
|
||||
}
|
||||
|
||||
function extractMarkdownSection(document: string, headingName: string): string {
|
||||
const heading = `## ${headingName}`;
|
||||
const start = document.indexOf(heading);
|
||||
if (start === -1) {
|
||||
return "";
|
||||
}
|
||||
|
||||
const afterHeading = start + heading.length;
|
||||
const nextH2 = document.indexOf("\n## ", afterHeading);
|
||||
const nextH1 = document.indexOf("\n# ", afterHeading);
|
||||
const endCandidates = [nextH2, nextH1].filter((value) => value !== -1);
|
||||
const end = endCandidates.length > 0 ? Math.min(...endCandidates) : document.length;
|
||||
|
||||
return document.slice(start, end).trim();
|
||||
}
|
||||
|
||||
function compactTaskPromptStepsSection(section: string): string {
|
||||
const stepTitles = Array.from(section.matchAll(/^### Step \d+:.*$/gm), (match) => match[0].trim());
|
||||
if (stepTitles.length === 0) {
|
||||
return section.trim();
|
||||
}
|
||||
|
||||
return [
|
||||
"## Steps",
|
||||
...stepTitles,
|
||||
"",
|
||||
"_... step checklist details trimmed for context limits ..._",
|
||||
].join("\n").trim();
|
||||
}
|
||||
|
||||
function truncateCompactedSection(section: string, maxChars: number, label: string): string {
|
||||
const trimmed = section.trim();
|
||||
if (!trimmed || trimmed.length <= maxChars) {
|
||||
return trimmed;
|
||||
}
|
||||
|
||||
const marker = `_... ${label} trimmed for context limits ..._`;
|
||||
const headBudget = Math.max(200, maxChars - marker.length - 2);
|
||||
return [
|
||||
`${trimmed.slice(0, headBudget).trimEnd()}…`,
|
||||
"",
|
||||
marker,
|
||||
].join("\n").trim();
|
||||
}
|
||||
|
||||
function compactTaskPromptSectionBody(body: string): string {
|
||||
const trimmed = body.trim();
|
||||
if (trimmed.length <= MAX_COMPACTED_TASK_PROMPT_CHARS) {
|
||||
return trimmed;
|
||||
}
|
||||
|
||||
const fencedMatch = /^```markdown\s*\n([\s\S]*?)\n```$/m.exec(trimmed);
|
||||
const promptContent = fencedMatch ? fencedMatch[1].trim() : trimmed;
|
||||
const firstSectionIndex = promptContent.indexOf("\n## ");
|
||||
const preamble = (firstSectionIndex === -1 ? promptContent : promptContent.slice(0, firstSectionIndex)).trim();
|
||||
const missionSection = extractMarkdownSection(promptContent, "Mission");
|
||||
const dependenciesSection = extractMarkdownSection(promptContent, "Dependencies");
|
||||
const fileScopeSection = extractMarkdownSection(promptContent, "File Scope");
|
||||
const stepsSection = compactTaskPromptStepsSection(extractMarkdownSection(promptContent, "Steps"));
|
||||
|
||||
const compactedContent = [
|
||||
truncateCompactedSection(preamble, 400, "task header"),
|
||||
truncateCompactedSection(missionSection, 900, "mission"),
|
||||
truncateCompactedSection(dependenciesSection, 500, "dependencies"),
|
||||
truncateCompactedSection(fileScopeSection, 1_000, "file scope"),
|
||||
truncateCompactedSection(stepsSection, 1_200, "steps outline"),
|
||||
"_... remaining PROMPT.md sections trimmed for context limits ..._",
|
||||
].filter(Boolean).join("\n\n").trim();
|
||||
|
||||
const narrowedContent = compactedContent.length <= MAX_COMPACTED_TASK_PROMPT_CHARS
|
||||
? compactedContent
|
||||
: compactExistingSpecificationSectionBody(compactedContent);
|
||||
const finalContent = fencedMatch ? `\`\`\`markdown\n${narrowedContent}\n\`\`\`` : narrowedContent;
|
||||
|
||||
return finalContent.length < trimmed.length ? finalContent : compactExistingSpecificationSectionBody(trimmed);
|
||||
}
|
||||
|
||||
function compactUserCommentsSectionBody(body: string): string {
|
||||
const trimmed = body.trim();
|
||||
if (trimmed.length <= MAX_COMPACTED_USER_COMMENTS_CHARS) {
|
||||
@@ -536,7 +614,7 @@ function compactUserCommentsSectionBody(body: string): string {
|
||||
}
|
||||
|
||||
function compactLargePromptSections(prompt: string): string | null {
|
||||
const sectionPattern = /(^|\n)(## (?:Subtask Consideration|Subtask Breakdown Requested|Attachments|Existing Specification|User Comments)\n)([\s\S]*?)(?=\n## [^#]|\n# [^#]|$)/g;
|
||||
const sectionPattern = /(^|\n)(## (?:Subtask Consideration|Subtask Breakdown Requested|Attachments|Existing Specification|Task PROMPT\.md|User Comments)\n)((?:\n*```markdown[\s\S]*?\n```|[\s\S]*?))(?=\n## [^#]|\n# [^#]|$)/g;
|
||||
let changed = false;
|
||||
|
||||
const compactedPrompt = prompt.replace(sectionPattern, (match, prefix: string, heading: string, body: string) => {
|
||||
@@ -548,6 +626,7 @@ function compactLargePromptSections(prompt: string): string | null {
|
||||
"Subtask Breakdown Requested": MAX_COMPACTED_SUBTASK_GUIDANCE_CHARS,
|
||||
Attachments: MAX_COMPACTED_ATTACHMENTS_CHARS,
|
||||
"Existing Specification": MAX_COMPACTED_EXISTING_SPEC_CHARS,
|
||||
"Task PROMPT.md": MAX_COMPACTED_TASK_PROMPT_CHARS,
|
||||
"User Comments": MAX_COMPACTED_USER_COMMENTS_CHARS,
|
||||
};
|
||||
|
||||
@@ -563,6 +642,8 @@ function compactLargePromptSections(prompt: string): string | null {
|
||||
compactedBody = compactAttachmentSectionBody(trimmedBody);
|
||||
} else if (headingName === "Existing Specification") {
|
||||
compactedBody = compactExistingSpecificationSectionBody(trimmedBody);
|
||||
} else if (headingName === "Task PROMPT.md") {
|
||||
compactedBody = compactTaskPromptSectionBody(trimmedBody);
|
||||
} else if (headingName === "User Comments") {
|
||||
compactedBody = compactUserCommentsSectionBody(trimmedBody);
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@
|
||||
import type { TaskStore, TaskComment, AgentPromptsConfig, Settings } from "@fusion/core";
|
||||
import { buildReviewerMemoryInstructions, resolveAgentPrompt } from "@fusion/core";
|
||||
import { describeModel, promptWithFallback } from "./pi.js";
|
||||
import { isContextLimitError } from "./context-limit-detector.js";
|
||||
import { createResolvedAgentSession, extractRuntimeHint } from "./agent-session-helpers.js";
|
||||
import { buildSessionSkillContext } from "./session-skill-context.js";
|
||||
import { AgentLogger } from "./agent-logger.js";
|
||||
@@ -336,12 +337,10 @@ export async function reviewStep(
|
||||
};
|
||||
}
|
||||
|
||||
// Build the review request
|
||||
const request = buildReviewRequest(
|
||||
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline, options.userComments,
|
||||
);
|
||||
|
||||
// Create AgentLogger for reviewer if store is available
|
||||
const agentLogger = options.store && options.taskId
|
||||
? new AgentLogger({
|
||||
store: options.store,
|
||||
@@ -355,12 +354,6 @@ export async function reviewStep(
|
||||
})
|
||||
: null;
|
||||
|
||||
// Resolve validator model settings using canonical lane hierarchy:
|
||||
// 1. Task-level validator override pair (taskValidatorProvider + taskValidatorModelId)
|
||||
// 2. Project-level validator override pair (projectValidatorProvider + projectValidatorModelId)
|
||||
// 3. Global validator lane pair (globalValidatorProvider + globalValidatorModelId)
|
||||
// 4. Project default override pair (projectDefaultOverrideProvider + projectDefaultOverrideModelId)
|
||||
// 5. Execution default pair (defaultProvider + defaultModelId)
|
||||
const validatorProvider = options.taskValidatorProvider && options.taskValidatorModelId
|
||||
? options.taskValidatorProvider
|
||||
: (options.projectValidatorProvider && options.projectValidatorModelId
|
||||
@@ -380,9 +373,6 @@ export async function reviewStep(
|
||||
? options.projectDefaultOverrideModelId
|
||||
: options.defaultModelId)));
|
||||
|
||||
// Resolve validator fallback using lane hierarchy:
|
||||
// 1. Project-level validator fallback (projectValidatorFallbackProvider + projectValidatorFallbackModelId)
|
||||
// 2. Execution fallback (fallbackProvider + fallbackModelId)
|
||||
const validatorFallbackProvider = options.projectValidatorFallbackProvider && options.projectValidatorFallbackModelId
|
||||
? options.projectValidatorFallbackProvider
|
||||
: options.fallbackProvider;
|
||||
@@ -390,7 +380,6 @@ export async function reviewStep(
|
||||
? options.projectValidatorFallbackModelId
|
||||
: options.fallbackModelId;
|
||||
|
||||
// Resolve per-agent custom instructions for the reviewer role
|
||||
let reviewerInstructions = "";
|
||||
if (options.agentStore && options.rootDir) {
|
||||
try {
|
||||
@@ -406,18 +395,10 @@ export async function reviewStep(
|
||||
}
|
||||
}
|
||||
const reviewerBasePrompt = resolveAgentPrompt("reviewer", options.agentPrompts) || REVIEWER_SYSTEM_PROMPT;
|
||||
// Memory goes in the dynamic layer (not concatenated onto basePrompt) so the
|
||||
// stable prefix is byte-identical across sessions even if memory changes.
|
||||
// The leading "\n" separator is no longer needed — buildPromptLayers handles
|
||||
// section joining with "\n\n".
|
||||
const memorySection = options.rootDir && options.settings?.memoryEnabled !== false
|
||||
? buildReviewerMemoryInstructions(options.rootDir, options.settings)
|
||||
: "";
|
||||
|
||||
// Build structured layers for cross-session prompt caching.
|
||||
// The stable layer (base prompt only) is byte-identical across all
|
||||
// reviewer sessions in this task, enabling cache hits. Memory goes
|
||||
// into the dynamic layer because it can change between sessions.
|
||||
const reviewerPluginContributions = buildPluginPromptSection(
|
||||
"reviewer",
|
||||
options.pluginRunner,
|
||||
@@ -432,12 +413,8 @@ export async function reviewStep(
|
||||
memorySection,
|
||||
pluginContributions: reviewerPluginContributions,
|
||||
});
|
||||
|
||||
// Collapsed string for backward compatibility with runtimes that don't
|
||||
// support layers (plugin runtimes, older pi versions).
|
||||
const reviewerSystemPromptFinal = collapsePromptLayers(layers);
|
||||
|
||||
// Build skill selection context (assigned agent skills take precedence over role fallback)
|
||||
let skillContext = undefined;
|
||||
if (options.agentStore && options.rootDir) {
|
||||
try {
|
||||
@@ -453,7 +430,6 @@ export async function reviewStep(
|
||||
}
|
||||
}
|
||||
|
||||
// Spawn a reviewer agent with read-only tools
|
||||
const assignedAgentId = options.task?.assignedAgentId ?? null;
|
||||
const agentStore = options.agentStore;
|
||||
const memoryAgent =
|
||||
@@ -481,26 +457,42 @@ export async function reviewStep(
|
||||
} : undefined),
|
||||
]
|
||||
: undefined;
|
||||
// Sentinel error used by the beforeCreateSession hook to cancel session
|
||||
// creation when a pause is detected after runtime resolution but before
|
||||
// the LLM session is actually spawned. Caught locally and converted to an
|
||||
// UNAVAILABLE verdict.
|
||||
class ReviewerPauseAbortError extends Error {
|
||||
constructor(public readonly reason: string) {
|
||||
super(`reviewer aborted: ${reason}`);
|
||||
}
|
||||
}
|
||||
|
||||
// Reviewers run within the parent agent's slot accounting via
|
||||
// semaphore.runNested at the call site. The session spawn itself includes
|
||||
// a last-chance pause check (beforeSpawnSession) that the runtime fires
|
||||
// immediately before the underlying LLM session is instantiated — past
|
||||
// every awaited setup step inside the runtime (provider registration,
|
||||
// resource loading, etc.). This closes the TOCTOU window where a pause
|
||||
// flipped during the setup chain.
|
||||
let session: import("@mariozechner/pi-coding-agent").AgentSession;
|
||||
try {
|
||||
({ session } = await createResolvedAgentSession({
|
||||
const activeSessions = new Set<import("@mariozechner/pi-coding-agent").AgentSession>();
|
||||
let reviewText = "";
|
||||
|
||||
const endSession = (session: import("@mariozechner/pi-coding-agent").AgentSession) => {
|
||||
if (!activeSessions.delete(session)) {
|
||||
return;
|
||||
}
|
||||
session.dispose();
|
||||
options.onSessionEnded?.(session);
|
||||
};
|
||||
|
||||
const buildPauseUnavailableResult = async (reason: string): Promise<ReviewResult> => {
|
||||
reviewerLog.log(
|
||||
`${taskId}: ${reviewType} review for Step ${stepNumber} aborted before spawn — ${reason} active`,
|
||||
);
|
||||
if (options.store && options.taskId) {
|
||||
await options.store.logEntry(
|
||||
options.taskId,
|
||||
`${reviewType} review aborted before spawn — ${reason} active`,
|
||||
).catch(() => undefined);
|
||||
}
|
||||
return {
|
||||
verdict: "UNAVAILABLE",
|
||||
review: `${reason} active — reviewer not spawned. Stop calling fn_review_* and exit cleanly; the parent task will resume after unpause.`,
|
||||
summary: `Skipped: ${reason}`,
|
||||
};
|
||||
};
|
||||
|
||||
const createReviewerSession = async (): Promise<import("@mariozechner/pi-coding-agent").AgentSession> => {
|
||||
const { session } = await createResolvedAgentSession({
|
||||
sessionPurpose: "reviewer",
|
||||
runtimeHint: extractRuntimeHint(memoryAgent?.runtimeConfig),
|
||||
pluginRunner: options.pluginRunner,
|
||||
@@ -518,7 +510,6 @@ export async function reviewStep(
|
||||
fallbackProvider: validatorFallbackProvider,
|
||||
fallbackModelId: validatorFallbackModelId,
|
||||
defaultThinkingLevel: options.defaultThinkingLevel,
|
||||
// Skill selection: use assigned agent skills if available, otherwise role fallback
|
||||
...(skillContext?.skillSelectionContext ? { skillSelection: skillContext.skillSelectionContext } : {}),
|
||||
taskId: options.taskId,
|
||||
taskTitle: options.taskTitle,
|
||||
@@ -535,8 +526,6 @@ export async function reviewStep(
|
||||
try {
|
||||
finalSettings = await options.store.getSettings();
|
||||
} catch {
|
||||
// Treat a transient store failure as "not paused" — better to
|
||||
// proceed than to block reviews on a flaky read.
|
||||
return;
|
||||
}
|
||||
if (finalSettings?.globalPause || finalSettings?.enginePaused) {
|
||||
@@ -544,61 +533,93 @@ export async function reviewStep(
|
||||
throw new ReviewerPauseAbortError(reason);
|
||||
}
|
||||
},
|
||||
}));
|
||||
});
|
||||
|
||||
const reviewerModelDesc = describeModel(session);
|
||||
const reviewerModelMarker = `Reviewer using model: ${reviewerModelDesc}`;
|
||||
reviewerLog.log(`${taskId}: reviewer using model ${reviewerModelDesc}`);
|
||||
if (options.store && options.taskId) {
|
||||
await options.store.logEntry(options.taskId, reviewerModelMarker);
|
||||
await options.store.appendAgentLog(options.taskId, reviewerModelMarker, "text", undefined, "reviewer").catch(() => undefined);
|
||||
}
|
||||
|
||||
activeSessions.add(session);
|
||||
options.onSessionCreated?.(session);
|
||||
session.subscribe((event) => {
|
||||
if (event.type === "message_update" && event.assistantMessageEvent.type === "text_delta") {
|
||||
reviewText += event.assistantMessageEvent.delta;
|
||||
}
|
||||
});
|
||||
|
||||
return session;
|
||||
};
|
||||
|
||||
const runReviewPrompt = async (
|
||||
session: import("@mariozechner/pi-coding-agent").AgentSession,
|
||||
prompt: string,
|
||||
): Promise<void> => {
|
||||
await promptWithFallback(session, prompt);
|
||||
checkSessionError(session);
|
||||
};
|
||||
|
||||
let session: import("@mariozechner/pi-coding-agent").AgentSession;
|
||||
try {
|
||||
session = await createReviewerSession();
|
||||
} catch (err) {
|
||||
if (err instanceof ReviewerPauseAbortError) {
|
||||
reviewerLog.log(
|
||||
`${taskId}: ${reviewType} review for Step ${stepNumber} aborted before spawn — ${err.reason} active`,
|
||||
);
|
||||
if (options.store && options.taskId) {
|
||||
await options.store.logEntry(
|
||||
options.taskId,
|
||||
`${reviewType} review aborted before spawn — ${err.reason} active`,
|
||||
).catch(() => undefined);
|
||||
}
|
||||
return {
|
||||
verdict: "UNAVAILABLE",
|
||||
review: `${err.reason} active — reviewer not spawned. Stop calling fn_review_* and exit cleanly; the parent task will resume after unpause.`,
|
||||
summary: `Skipped: ${err.reason}`,
|
||||
};
|
||||
return buildPauseUnavailableResult(err.reason);
|
||||
}
|
||||
throw err;
|
||||
}
|
||||
|
||||
const reviewerModelDesc = describeModel(session);
|
||||
const reviewerModelMarker = `Reviewer using model: ${reviewerModelDesc}`;
|
||||
reviewerLog.log(`${taskId}: reviewer using model ${reviewerModelDesc}`);
|
||||
if (options.store && options.taskId) {
|
||||
await options.store.logEntry(options.taskId, reviewerModelMarker);
|
||||
await options.store.appendAgentLog(options.taskId, reviewerModelMarker, "text", undefined, "reviewer").catch(() => undefined);
|
||||
}
|
||||
|
||||
// Notify the caller so it can track this session in a per-task subagent map.
|
||||
// If the parent task is later moved out of in-progress, paused, or the engine
|
||||
// is globally paused, the caller will dispose this session — preventing the
|
||||
// reviewer from outliving its parent task.
|
||||
options.onSessionCreated?.(session);
|
||||
|
||||
let reviewText = "";
|
||||
|
||||
// Capture the reviewer's full text output (still needed for verdict extraction)
|
||||
session.subscribe((event) => {
|
||||
if (event.type === "message_update" && event.assistantMessageEvent.type === "text_delta") {
|
||||
reviewText += event.assistantMessageEvent.delta;
|
||||
}
|
||||
});
|
||||
|
||||
try {
|
||||
await promptWithFallback(session, request);
|
||||
try {
|
||||
await runReviewPrompt(session, request);
|
||||
} catch (err: unknown) {
|
||||
const errorMessage = err instanceof Error ? err.message : String(err);
|
||||
if (!isContextLimitError(errorMessage)) {
|
||||
throw err;
|
||||
}
|
||||
|
||||
// Re-raise errors that pi-coding-agent swallowed after exhausting retries.
|
||||
// The caller (executor's createReviewStepTool) catches errors and returns
|
||||
// UNAVAILABLE, so the thrown error will be handled there.
|
||||
checkSessionError(session);
|
||||
const retryLogMessage = reviewType === "code"
|
||||
? "code review hit context limit — retrying with compacted request"
|
||||
: `${reviewType} review hit context limit — retrying with compacted request`;
|
||||
reviewerLog.warn(`${taskId}: ${retryLogMessage}`);
|
||||
if (options.store && options.taskId) {
|
||||
await options.store.logEntry(options.taskId, retryLogMessage).catch(() => undefined);
|
||||
}
|
||||
|
||||
reviewText = "";
|
||||
const reducedRequest = buildReducedReviewRequest(
|
||||
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline,
|
||||
);
|
||||
|
||||
try {
|
||||
await runReviewPrompt(session, reducedRequest);
|
||||
} catch (retryErr: unknown) {
|
||||
if (!isReviewerSessionReuseError(retryErr)) {
|
||||
throw retryErr;
|
||||
}
|
||||
|
||||
endSession(session);
|
||||
try {
|
||||
session = await createReviewerSession();
|
||||
} catch (recreateErr) {
|
||||
if (recreateErr instanceof ReviewerPauseAbortError) {
|
||||
return buildPauseUnavailableResult(recreateErr.reason);
|
||||
}
|
||||
throw recreateErr;
|
||||
}
|
||||
await runReviewPrompt(session, reducedRequest);
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
if (agentLogger) await agentLogger.flush();
|
||||
session.dispose();
|
||||
options.onSessionEnded?.(session);
|
||||
if (agentLogger) {
|
||||
await agentLogger.flush();
|
||||
}
|
||||
for (const activeSession of [...activeSessions]) {
|
||||
endSession(activeSession);
|
||||
}
|
||||
}
|
||||
|
||||
const verdict = extractVerdict(reviewText);
|
||||
@@ -606,6 +627,71 @@ export async function reviewStep(
|
||||
return { verdict, review: reviewText, summary };
|
||||
}
|
||||
|
||||
function isReviewerSessionReuseError(error: unknown): boolean {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
return /prompt is in progress|session (?:is )?(?:closed|disposed|ended)|conversation already active/i.test(message);
|
||||
}
|
||||
|
||||
function extractPromptSection(promptContent: string, sectionName: string): string {
|
||||
const heading = `## ${sectionName}`;
|
||||
const start = promptContent.indexOf(heading);
|
||||
if (start === -1) {
|
||||
return "";
|
||||
}
|
||||
|
||||
const afterHeading = start + heading.length;
|
||||
const nextH2 = promptContent.indexOf("\n## ", afterHeading);
|
||||
const nextH1 = promptContent.indexOf("\n# ", afterHeading);
|
||||
const endCandidates = [nextH2, nextH1].filter((value) => value !== -1);
|
||||
const end = endCandidates.length > 0 ? Math.min(...endCandidates) : promptContent.length;
|
||||
return promptContent.slice(start, end).trim();
|
||||
}
|
||||
|
||||
function summarizePromptSteps(promptContent: string): string {
|
||||
const stepTitles = Array.from(promptContent.matchAll(/^### Step \d+:.*$/gm), (match) => match[0].trim());
|
||||
if (stepTitles.length === 0) {
|
||||
return "";
|
||||
}
|
||||
|
||||
return ["## Steps", ...stepTitles].join("\n");
|
||||
}
|
||||
|
||||
function buildReducedTaskPromptSummary(promptContent: string): string {
|
||||
const firstSectionIndex = promptContent.indexOf("\n## ");
|
||||
const header = (firstSectionIndex === -1 ? promptContent : promptContent.slice(0, firstSectionIndex)).trim();
|
||||
const sections = [
|
||||
header,
|
||||
extractPromptSection(promptContent, "Mission"),
|
||||
extractPromptSection(promptContent, "Dependencies"),
|
||||
extractPromptSection(promptContent, "File Scope"),
|
||||
summarizePromptSteps(promptContent),
|
||||
"_... additional PROMPT.md sections omitted after context-limit retry ..._",
|
||||
].filter(Boolean);
|
||||
|
||||
return sections.join("\n\n").trim();
|
||||
}
|
||||
|
||||
function buildReducedReviewRequest(
|
||||
taskId: string,
|
||||
stepNumber: number,
|
||||
stepName: string,
|
||||
reviewType: ReviewType,
|
||||
promptContent: string,
|
||||
cwd: string,
|
||||
baseline?: string,
|
||||
): string {
|
||||
return buildReviewRequest(
|
||||
taskId,
|
||||
stepNumber,
|
||||
stepName,
|
||||
reviewType,
|
||||
buildReducedTaskPromptSummary(promptContent),
|
||||
cwd,
|
||||
baseline,
|
||||
undefined,
|
||||
);
|
||||
}
|
||||
|
||||
function buildReviewRequest(
|
||||
taskId: string,
|
||||
stepNumber: number,
|
||||
|
||||
Reference in New Issue
Block a user