FN-7294: pass user context into workflow reviewers
Pass operator-authored task context into reviewer and workflow gate prompts. - Include de-duped user comments and legacy steering in Plan Review, code review, and workflow-step prompt agents. - Preserve reviewer comment context on reduced context-limit retry prompts and refresh comments before in-session review calls. - Document workflow prompt-agent comment behavior, add regression coverage, and add a patch changeset. Files changed: .changeset/FN-7294-reviewer-comment-context.md | 7 + docs/workflow-steps.md | 7 + .../src/__tests__/agent-user-comments.test.ts | 81 ++++++++-- .../executor-review-step-indexing.test.ts | 165 ++++++++++++++++++++- .../src/__tests__/reviewer-workspace.test.ts | 39 +++++ packages/engine/src/__tests__/reviewer.test.ts | 26 +++- packages/engine/src/__tests__/triage.test.ts | 49 ++++++ packages/engine/src/agent-user-comments.ts | 42 ++++-- packages/engine/src/executor.ts | 31 +++- packages/engine/src/reviewer.ts | 10 +- packages/engine/src/triage.ts | 20 ++- 11 files changed, 443 insertions(+), 34 deletions(-) Fusion-Task-Id: FN-7294 Fusion-Task-Lineage: ea5ff33e-99ba-4fdd-9257-9778849b91bb Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
7
.changeset/FN-7294-reviewer-comment-context.md
Normal file
7
.changeset/FN-7294-reviewer-comment-context.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Review gates now include user comments and steering context consistently.
|
||||
category: fix
|
||||
dev: Mandatory Plan Review, reviewStep callers, and prompt/custom workflow-step agents pass canonical user comment context.
|
||||
@@ -421,6 +421,13 @@ Post-merge runs **graph-native**: after a successful merge the executor continue
|
||||
|
||||
Prompt mode can run with readonly or coding-capable tool access depending on step/template configuration.
|
||||
|
||||
<!--
|
||||
FNXC:AgentSteering 2026-06-30-14:18:
|
||||
Workflow-step prompt agents are reviewer-style gates even when they do not call the shared reviewStep helper. They receive the same canonical user-authored task comments and legacy steering context as Plan Review and Code Review so Browser Verification and custom gates evaluate explicit operator requirements.
|
||||
-->
|
||||
|
||||
Prompt-mode workflow-step agents receive user-authored task comments plus legacy steering entries in their system prompt. Agent-authored comments are excluded, duplicate ids are de-duped, and this context is omitted when no user-authored entries exist.
|
||||
|
||||
## Tool Modes
|
||||
|
||||
`toolMode: "readonly"` is enforced as a hard session-level allowlist. Readonly workflow-step agents can only access:
|
||||
|
||||
@@ -48,34 +48,89 @@ describe("agent user comments prompt helper", () => {
|
||||
expect(section).toContain("> Please keep the old API export");
|
||||
});
|
||||
|
||||
it("dedupes duplicate ids", () => {
|
||||
it("selects user-authored legacy steering comments alongside unified comments", () => {
|
||||
const selected = selectUserCommentsForAgentContext({
|
||||
comments: [
|
||||
comment({ id: "dup", text: "old duplicate", createdAt: "2026-06-21T10:00:00.000Z" }),
|
||||
comment({ id: "dup", text: "new duplicate", createdAt: "2026-06-21T11:00:00.000Z" }),
|
||||
comments: [comment({ id: "c-user", text: "Unified user requirement", createdAt: "2026-06-21T10:00:00.000Z" })],
|
||||
steeringComments: [
|
||||
{ id: "s-user", text: "Legacy steering requirement", author: "user", createdAt: "2026-06-21T10:05:00.000Z" },
|
||||
{ id: "s-agent", text: "agent-only steering", author: "agent", createdAt: "2026-06-21T10:06:00.000Z" },
|
||||
],
|
||||
});
|
||||
|
||||
expect(selected.map((c) => c.id)).toEqual(["c-user", "s-user"]);
|
||||
const section = buildUserCommentsPromptSection(selected);
|
||||
expect(section).toContain("Unified user requirement");
|
||||
expect(section).toContain("Legacy steering requirement");
|
||||
expect(section).not.toContain("agent-only steering");
|
||||
});
|
||||
|
||||
it("dedupes duplicate ids across unified comments and legacy steering comments", () => {
|
||||
const selected = selectUserCommentsForAgentContext({
|
||||
comments: [comment({ id: "dup", text: "unified duplicate", createdAt: "2026-06-21T10:00:00.000Z" })],
|
||||
steeringComments: [
|
||||
{ id: "dup", text: "steering duplicate wins", author: "user", createdAt: "2026-06-21T11:00:00.000Z" },
|
||||
],
|
||||
});
|
||||
|
||||
const section = buildUserCommentsPromptSection(selected);
|
||||
|
||||
expect(selected).toHaveLength(1);
|
||||
expect(section).toContain("new duplicate");
|
||||
expect(section).not.toContain("old duplicate");
|
||||
expect(section).toContain("steering duplicate wins");
|
||||
expect(section).not.toContain("unified duplicate");
|
||||
});
|
||||
|
||||
it("caps a large history to the newest comments in chronological order", () => {
|
||||
const comments = Array.from({ length: 25 }, (_, index) => comment({
|
||||
it("preserves multiline text when formatting selected comments and steering", () => {
|
||||
const selected = selectUserCommentsForAgentContext({
|
||||
steeringComments: [
|
||||
{ id: "s-multiline", text: "Line one\nLine two", author: "user", createdAt: "2026-06-21T10:00:00.000Z" },
|
||||
],
|
||||
});
|
||||
|
||||
expect(buildUserCommentsPromptSection(selected)).toContain("> Line one\n> Line two");
|
||||
});
|
||||
|
||||
it("caps a large mixed history to the requested newest comments in chronological order", () => {
|
||||
const comments = Array.from({ length: 15 }, (_, index) => comment({
|
||||
id: `user-${index}`,
|
||||
text: `comment ${index}`,
|
||||
createdAt: `2026-06-21T10:${String(index).padStart(2, "0")}:00.000Z`,
|
||||
}));
|
||||
const steeringComments = Array.from({ length: 15 }, (_, index) => ({
|
||||
id: `steer-${index}`,
|
||||
text: `steering ${index}`,
|
||||
author: "user" as const,
|
||||
createdAt: `2026-06-21T11:${String(index).padStart(2, "0")}:00.000Z`,
|
||||
}));
|
||||
|
||||
const selected = selectUserCommentsForAgentContext({ comments }, { limit: 3 });
|
||||
const selected = selectUserCommentsForAgentContext({ comments, steeringComments }, { limit: 3 });
|
||||
const section = buildUserCommentsPromptSection(selected);
|
||||
|
||||
expect(selected.map((c) => c.id)).toEqual(["user-22", "user-23", "user-24"]);
|
||||
expect(section).not.toContain("comment 21");
|
||||
expect(section.indexOf("comment 22")).toBeLessThan(section.indexOf("comment 23"));
|
||||
expect(section.indexOf("comment 23")).toBeLessThan(section.indexOf("comment 24"));
|
||||
expect(selected.map((c) => c.id)).toEqual(["steer-12", "steer-13", "steer-14"]);
|
||||
expect(section).not.toContain("steering 11");
|
||||
expect(section.indexOf("steering 12")).toBeLessThan(section.indexOf("steering 13"));
|
||||
expect(section.indexOf("steering 13")).toBeLessThan(section.indexOf("steering 14"));
|
||||
});
|
||||
|
||||
it("returns all user comments and steering entries when reviewer callers request uncapped context", () => {
|
||||
const comments = Array.from({ length: 15 }, (_, index) => comment({
|
||||
id: `user-${index}`,
|
||||
text: `comment ${index}`,
|
||||
createdAt: `2026-06-21T10:${String(index).padStart(2, "0")}:00.000Z`,
|
||||
}));
|
||||
const steeringComments = Array.from({ length: 15 }, (_, index) => ({
|
||||
id: `steer-${index}`,
|
||||
text: `steering ${index}`,
|
||||
author: "user" as const,
|
||||
createdAt: `2026-06-21T11:${String(index).padStart(2, "0")}:00.000Z`,
|
||||
}));
|
||||
|
||||
const selected = selectUserCommentsForAgentContext({ comments, steeringComments }, { limit: null });
|
||||
const section = buildUserCommentsPromptSection(selected);
|
||||
|
||||
expect(selected).toHaveLength(30);
|
||||
expect(selected[0]?.id).toBe("user-0");
|
||||
expect(selected.at(-1)?.id).toBe("steer-14");
|
||||
expect(section).toContain("comment 0");
|
||||
expect(section).toContain("steering 14");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -11,7 +11,7 @@ import {
|
||||
|
||||
const mockedReviewStep = vi.mocked(mockedReviewStepFn);
|
||||
|
||||
async function captureTools(comments: any[] = []) {
|
||||
async function captureTools(comments: any[] = [], steeringComments: any[] = []) {
|
||||
const store = createMockStore();
|
||||
const stepStates = [
|
||||
{ name: "Preflight", status: "done" },
|
||||
@@ -34,6 +34,7 @@ async function captureTools(comments: any[] = []) {
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
comments,
|
||||
steeringComments,
|
||||
}));
|
||||
store.updateStep.mockImplementation(async (_taskId: string, stepIndex: number, status: string) => {
|
||||
stepStates[stepIndex].status = status;
|
||||
@@ -76,6 +77,126 @@ async function captureTools(comments: any[] = []) {
|
||||
return { tools, store, stepStates, navigateTree, setLeaf: (leaf: string) => { checkpointLeafId = leaf; } };
|
||||
}
|
||||
|
||||
function workflowPromptTask(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: "FN-WF-PROMPT",
|
||||
title: "Workflow prompt",
|
||||
description: "Run reviewer workflow prompt",
|
||||
column: "in-progress" as const,
|
||||
worktree: "/tmp/wt",
|
||||
branch: "fusion/fn-wf-prompt",
|
||||
baseCommitSha: "abc123",
|
||||
dependencies: [],
|
||||
steps: [{ name: "Done", status: "done" as const }],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
prompt: "# prompt",
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function workflowPromptStep(overrides: Record<string, unknown> = {}) {
|
||||
return {
|
||||
id: "graph:browser-verification-step",
|
||||
name: "Browser Verification",
|
||||
description: "",
|
||||
mode: "prompt",
|
||||
phase: "pre-merge",
|
||||
gateMode: "gate",
|
||||
prompt: "Verify the operator requirements.",
|
||||
toolMode: "readonly",
|
||||
enabled: true,
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
async function captureWorkflowStepSystemPrompt(taskDetail: Record<string, unknown>) {
|
||||
const store = createMockStore();
|
||||
store.getTask.mockResolvedValue(taskDetail as any);
|
||||
const captured: { systemPrompt?: string } = {};
|
||||
mockedCreateFnAgent.mockImplementation(async (opts: any) => {
|
||||
captured.systemPrompt = opts.systemPrompt;
|
||||
const listeners: Array<(event: any) => void> = [];
|
||||
return {
|
||||
session: {
|
||||
state: {},
|
||||
subscribe: (fn: (event: any) => void) => {
|
||||
listeners.push(fn);
|
||||
return () => {};
|
||||
},
|
||||
prompt: vi.fn(async () => {
|
||||
for (const fn of listeners) {
|
||||
fn({
|
||||
type: "message_update",
|
||||
assistantMessageEvent: {
|
||||
type: "text_delta",
|
||||
partial: '{"verdict":"APPROVE","notes":""}',
|
||||
contentIndex: 0,
|
||||
delta: '{"verdict":"APPROVE","notes":""}',
|
||||
},
|
||||
});
|
||||
}
|
||||
}),
|
||||
dispose: vi.fn(),
|
||||
},
|
||||
} as any;
|
||||
});
|
||||
const executor = new TaskExecutor(store, "/tmp/test", {
|
||||
agentStore: { getAgent: vi.fn().mockResolvedValue(null), createAgent: vi.fn() },
|
||||
} as any);
|
||||
|
||||
await (executor as any).executeWorkflowStep(
|
||||
workflowPromptTask({ id: taskDetail.id }),
|
||||
workflowPromptStep(),
|
||||
"/tmp/wt",
|
||||
{},
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
|
||||
return captured.systemPrompt ?? "";
|
||||
}
|
||||
|
||||
describe("workflow prompt reviewer comment context", () => {
|
||||
beforeEach(() => {
|
||||
resetExecutorMocks();
|
||||
});
|
||||
|
||||
it("adds canonical user comments and legacy steering to workflow-step reviewer system prompts", async () => {
|
||||
const systemPrompt = await captureWorkflowStepSystemPrompt(workflowPromptTask({
|
||||
comments: [
|
||||
{ id: "c-user", text: "Unified workflow prompt requirement", author: "user", createdAt: "2026-06-21T10:00:00.000Z" },
|
||||
{ id: "c-agent", text: "agent-only workflow note", author: "agent", createdAt: "2026-06-21T10:01:00.000Z" },
|
||||
],
|
||||
steeringComments: [
|
||||
{ id: "s-user", text: "Legacy workflow prompt steering", author: "user", createdAt: "2026-06-21T10:02:00.000Z" },
|
||||
{ id: "s-agent", text: "agent-only legacy steering", author: "agent", createdAt: "2026-06-21T10:03:00.000Z" },
|
||||
],
|
||||
}));
|
||||
|
||||
expect(systemPrompt).toContain("## User Comments");
|
||||
expect(systemPrompt).toContain("Unified workflow prompt requirement");
|
||||
expect(systemPrompt).toContain("Legacy workflow prompt steering");
|
||||
expect(systemPrompt).not.toContain("agent-only workflow note");
|
||||
expect(systemPrompt).not.toContain("agent-only legacy steering");
|
||||
});
|
||||
|
||||
it("omits empty user comment sections for workflow-step reviewer system prompts", async () => {
|
||||
const systemPrompt = await captureWorkflowStepSystemPrompt(workflowPromptTask({
|
||||
comments: [{ id: "c-agent", text: "agent-only workflow note", author: "agent", createdAt: "2026-06-21T10:01:00.000Z" }],
|
||||
steeringComments: [{ id: "s-agent", text: "agent-only legacy steering", author: "agent", createdAt: "2026-06-21T10:03:00.000Z" }],
|
||||
}));
|
||||
|
||||
expect(systemPrompt).not.toContain("## User Comments");
|
||||
expect(systemPrompt).not.toContain("agent-only workflow note");
|
||||
expect(systemPrompt).not.toContain("agent-only legacy steering");
|
||||
});
|
||||
});
|
||||
|
||||
describe("fn_review_step indexing", () => {
|
||||
beforeEach(() => {
|
||||
resetExecutorMocks();
|
||||
@@ -113,9 +234,15 @@ describe("fn_review_step indexing", () => {
|
||||
expect(result.content[0].text).toContain("Cannot mark Step 1 as done");
|
||||
});
|
||||
|
||||
it("passes fresh user comments into reviewStep", async () => {
|
||||
it("passes fresh user comments and legacy steering into reviewStep", async () => {
|
||||
mockedReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "ok", summary: "ok" } as any);
|
||||
const { tools } = await captureTools([
|
||||
...Array.from({ length: 21 }, (_, index) => ({
|
||||
id: `c-old-${index}`,
|
||||
text: `Older in-session review requirement ${index}`,
|
||||
author: "user" as const,
|
||||
createdAt: `2026-06-21T09:${String(index).padStart(2, "0")}:00.000Z`,
|
||||
})),
|
||||
{
|
||||
id: "c-user",
|
||||
text: "Please keep the old API export",
|
||||
@@ -128,14 +255,46 @@ describe("fn_review_step indexing", () => {
|
||||
author: "agent",
|
||||
createdAt: "2026-06-21T11:00:00.000Z",
|
||||
},
|
||||
], [
|
||||
{
|
||||
id: "s-user",
|
||||
text: "Please preserve the steering requirement",
|
||||
author: "user",
|
||||
createdAt: "2026-06-21T10:30:00.000Z",
|
||||
},
|
||||
{
|
||||
id: "s-agent",
|
||||
text: "agent-only steering",
|
||||
author: "agent",
|
||||
createdAt: "2026-06-21T10:31:00.000Z",
|
||||
},
|
||||
]);
|
||||
|
||||
await tools.fn_review_step("call-1", { step: 1, type: "code", step_name: "Implement", baseline: "abc" });
|
||||
|
||||
const options = mockedReviewStep.mock.calls[0]?.[7] as any;
|
||||
expect(options.userComments).toEqual([
|
||||
expect(options.userComments).toHaveLength(23);
|
||||
expect(options.userComments).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ id: "c-old-0", text: "Older in-session review requirement 0", author: "user" }),
|
||||
expect.objectContaining({ id: "c-user", text: "Please keep the old API export", author: "user" }),
|
||||
expect.objectContaining({ id: "s-user", text: "Please preserve the steering requirement", author: "user" }),
|
||||
]));
|
||||
expect(options.userComments).not.toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ id: "c-agent" }),
|
||||
expect.objectContaining({ id: "s-agent" }),
|
||||
]));
|
||||
});
|
||||
|
||||
it("omits userComments when reviewStep has no user-authored context", async () => {
|
||||
mockedReviewStep.mockResolvedValue({ verdict: "APPROVE", review: "ok", summary: "ok" } as any);
|
||||
const { tools } = await captureTools([], [
|
||||
{ id: "s-agent", text: "agent-only steering", author: "agent", createdAt: "2026-06-21T10:31:00.000Z" },
|
||||
]);
|
||||
|
||||
await tools.fn_review_step("call-1", { step: 1, type: "code", step_name: "Implement", baseline: "abc" });
|
||||
|
||||
const options = mockedReviewStep.mock.calls[0]?.[7] as any;
|
||||
expect(options.userComments).toBeUndefined();
|
||||
});
|
||||
|
||||
it("rejects out-of-range steps without reviewer call", async () => {
|
||||
|
||||
@@ -271,6 +271,45 @@ describe("U2 KTD3 — step-inversion review seam (executor.ts:5668) loops per su
|
||||
expect(result.verdict).toBe("APPROVE");
|
||||
});
|
||||
|
||||
it("passes unified user comments and legacy steering into workflow graph stepReview", async () => {
|
||||
const task = makeTask({
|
||||
worktree: WT_A,
|
||||
comments: [
|
||||
...Array.from({ length: 21 }, (_, index) => ({
|
||||
id: `c-old-${index}`,
|
||||
text: `Older graph-review requirement ${index}`,
|
||||
author: "user" as const,
|
||||
createdAt: `2026-06-21T09:${String(index).padStart(2, "0")}:00.000Z`,
|
||||
})),
|
||||
{ id: "c-user", text: "Unified graph-review requirement", author: "user", createdAt: "2026-06-21T10:00:00.000Z" },
|
||||
{ id: "c-agent", text: "agent-only unified note", author: "agent", createdAt: "2026-06-21T10:01:00.000Z" },
|
||||
],
|
||||
steeringComments: [
|
||||
{ id: "s-user", text: "Legacy graph-review steering", author: "user", createdAt: "2026-06-21T10:02:00.000Z" },
|
||||
{ id: "s-agent", text: "agent-only steering note", author: "agent", createdAt: "2026-06-21T10:03:00.000Z" },
|
||||
],
|
||||
} as any);
|
||||
const store = makeStore(task);
|
||||
const executor = new TaskExecutor(store, ROOT);
|
||||
scriptReviewByCwd({ [WT_A]: { verdict: "APPROVE", review: "a", summary: "a" } });
|
||||
const seams = executor.createAuthoritativeWorkflowSeams({ autoMerge: false } as any);
|
||||
const context = { [FOREACH_ACTIVE_CONTEXT_KEY]: { stepIndex: 1, worktreePath: WT_A, baselineSha: "base" } } as any;
|
||||
|
||||
await seams.stepReview!(task as any, context, { type: "code", advisory: true } as any);
|
||||
|
||||
const options = mockedReviewStep.mock.calls[0]?.[7] as any;
|
||||
expect(options.userComments).toHaveLength(23);
|
||||
expect(options.userComments).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ id: "c-old-0", text: "Older graph-review requirement 0", author: "user" }),
|
||||
expect.objectContaining({ id: "c-user", text: "Unified graph-review requirement", author: "user" }),
|
||||
expect.objectContaining({ id: "s-user", text: "Legacy graph-review steering", author: "user" }),
|
||||
]));
|
||||
expect(options.userComments).not.toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ id: "c-agent" }),
|
||||
expect.objectContaining({ id: "s-agent" }),
|
||||
]));
|
||||
});
|
||||
|
||||
it("regression: single-repo stepReview reviews the active worktree once", async () => {
|
||||
const task = makeTask({ worktree: WT_A });
|
||||
const store = makeStore(task);
|
||||
|
||||
@@ -522,6 +522,16 @@ describe("reviewStep — context-limit retry", () => {
|
||||
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 userComments = [
|
||||
{
|
||||
id: "user-comment-1",
|
||||
text: "User says compact retry must keep this requirement.",
|
||||
author: "user" as const,
|
||||
createdAt: "2026-06-30T16:00:00.000Z",
|
||||
updatedAt: "2026-06-30T16:01:00.000Z",
|
||||
},
|
||||
];
|
||||
|
||||
const result = await reviewStep(
|
||||
"/tmp/worktree",
|
||||
"FN-4082",
|
||||
@@ -530,7 +540,7 @@ describe("reviewStep — context-limit retry", () => {
|
||||
"code",
|
||||
promptContent,
|
||||
"abc123",
|
||||
{ store: store as any, taskId: "FN-4082" },
|
||||
{ store: store as any, taskId: "FN-4082", userComments },
|
||||
);
|
||||
|
||||
expect(result.verdict).toBe("APPROVE");
|
||||
@@ -542,6 +552,9 @@ describe("reviewStep — context-limit retry", () => {
|
||||
expect(secondRequest).toContain("## Mission");
|
||||
expect(secondRequest).toContain("## File Scope");
|
||||
expect(secondRequest).toContain("### Step 1: Compact prompt");
|
||||
expect(secondRequest).toContain("## User Comments");
|
||||
expect(secondRequest).toContain("User says compact retry must keep this requirement.");
|
||||
expect(secondRequest.match(/## User Comments/g)).toHaveLength(1);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-4082",
|
||||
"code review hit context limit — retrying with compacted request",
|
||||
@@ -1084,6 +1097,12 @@ describe("reviewStep — user comments in spec review", () => {
|
||||
author: "user",
|
||||
createdAt: "2026-01-02T10:00:00.000Z",
|
||||
},
|
||||
{
|
||||
id: "s1",
|
||||
text: "Legacy steering: keep compatibility",
|
||||
author: "user",
|
||||
createdAt: "2026-01-02T10:05:00.000Z",
|
||||
},
|
||||
];
|
||||
|
||||
await reviewStep(
|
||||
@@ -1095,6 +1114,9 @@ describe("reviewStep — user comments in spec review", () => {
|
||||
|
||||
expect(capturedPrompt).toContain("User Comment Coverage (MANDATORY)");
|
||||
expect(capturedPrompt).toContain("Make sure to handle the edge case");
|
||||
expect(capturedPrompt).toContain("Legacy steering: keep compatibility");
|
||||
expect(capturedPrompt.match(/User Comment Coverage \(MANDATORY\)/g)).toHaveLength(1);
|
||||
expect(capturedPrompt).not.toContain("## User Comments");
|
||||
expect(capturedPrompt).toContain("issue a REVISE verdict");
|
||||
});
|
||||
|
||||
@@ -1121,6 +1143,7 @@ describe("reviewStep — user comments in spec review", () => {
|
||||
);
|
||||
|
||||
expect(capturedPrompt).not.toContain("User Comment Coverage");
|
||||
expect(capturedPrompt).not.toContain("## User Comments");
|
||||
});
|
||||
|
||||
it.each(["plan", "code"] as const)("includes user comments for %s reviews without spec coverage gating", async (reviewType) => {
|
||||
@@ -1158,6 +1181,7 @@ describe("reviewStep — user comments in spec review", () => {
|
||||
|
||||
expect(capturedPrompt).toContain("## User Comments");
|
||||
expect(capturedPrompt).toContain("Some user feedback");
|
||||
expect(capturedPrompt.match(/## User Comments/g)).toHaveLength(1);
|
||||
expect(capturedPrompt).not.toContain("User Comment Coverage (MANDATORY)");
|
||||
});
|
||||
|
||||
|
||||
@@ -1319,6 +1319,55 @@ describe("TriageProcessor", () => {
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-PLAN-APPROVE", "todo");
|
||||
});
|
||||
|
||||
it("passes fresh user comments and legacy steering into enabled Plan Review", async () => {
|
||||
const task = createTriageTask({
|
||||
id: "FN-PLAN-COMMENTS",
|
||||
title: "Plan comments",
|
||||
status: "planning",
|
||||
enabledWorkflowSteps: ["plan-review"],
|
||||
comments: [
|
||||
...Array.from({ length: 21 }, (_, index) => ({
|
||||
id: `c-old-${index}`,
|
||||
text: `Older reviewer requirement ${index}`,
|
||||
author: "user" as const,
|
||||
createdAt: `2026-06-21T09:${String(index).padStart(2, "0")}:00.000Z`,
|
||||
})),
|
||||
{ id: "c-user", text: "Unified reviewer requirement", author: "user", createdAt: "2026-06-21T10:00:00.000Z" },
|
||||
{ id: "c-agent", text: "agent-only unified note", author: "agent", createdAt: "2026-06-21T10:01:00.000Z" },
|
||||
],
|
||||
steeringComments: [
|
||||
{ id: "s-user", text: "Legacy steering reviewer requirement", author: "user", createdAt: "2026-06-21T10:02:00.000Z" },
|
||||
{ id: "s-agent", text: "agent-only steering note", author: "agent", createdAt: "2026-06-21T10:03:00.000Z" },
|
||||
],
|
||||
} as Partial<Task>);
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(task);
|
||||
mockReviewStep.mockResolvedValue({
|
||||
verdict: "APPROVE",
|
||||
review: "### Verdict: APPROVE\n\n### Summary\nReady.",
|
||||
summary: "Ready.",
|
||||
});
|
||||
|
||||
await (processor as unknown as {
|
||||
finalizeApprovedTask(task: Task, writtenInput: string, settings: Settings): Promise<void>;
|
||||
}).finalizeApprovedTask(
|
||||
task,
|
||||
"# Task: FN-PLAN-COMMENTS - Plan comments\n\n## Mission\n\nDo it.\n",
|
||||
{ requirePlanApproval: false } as Settings,
|
||||
);
|
||||
|
||||
const options = mockReviewStep.mock.calls[0]?.[7] as any;
|
||||
expect(options.userComments).toHaveLength(23);
|
||||
expect(options.userComments).toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ id: "c-old-0", text: "Older reviewer requirement 0", author: "user" }),
|
||||
expect.objectContaining({ id: "c-user", text: "Unified reviewer requirement", author: "user" }),
|
||||
expect.objectContaining({ id: "s-user", text: "Legacy steering reviewer requirement", author: "user" }),
|
||||
]));
|
||||
expect(options.userComments).not.toEqual(expect.arrayContaining([
|
||||
expect.objectContaining({ id: "c-agent" }),
|
||||
expect.objectContaining({ id: "s-agent" }),
|
||||
]));
|
||||
});
|
||||
|
||||
it("keeps the task in triage when Plan Review requests revision", async () => {
|
||||
const task = createTriageTask({
|
||||
id: "FN-PLAN-REVISE",
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import type { TaskComment } from "@fusion/core";
|
||||
import type { SteeringComment, TaskComment } from "@fusion/core";
|
||||
|
||||
const DEFAULT_USER_COMMENT_LIMIT = 20;
|
||||
|
||||
@@ -17,26 +17,48 @@ function quoteCommentText(text: string): string[] {
|
||||
return normalized.split(/\r?\n/).map((line) => `> ${line}`);
|
||||
}
|
||||
|
||||
function normalizeSteeringComment(comment: SteeringComment): TaskComment {
|
||||
return {
|
||||
id: comment.id,
|
||||
text: comment.text,
|
||||
author: comment.author,
|
||||
createdAt: comment.createdAt,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:AgentSteering 2026-06-22-00:05:
|
||||
* Task-detail chat and user comments must reach every agent lane that builds prompts: executor, merger, reviewer, and planner. This helper is the canonical formatter for next-prompt delivery outside the executor's live steering injection path, so merger and reviewer prompts do not drift or duplicate comment logic.
|
||||
*
|
||||
* FNXC:AgentSteering 2026-06-30-12:28:
|
||||
* Reviewer gates are quality gates for explicit operator requirements. Select unified comments plus legacy steering comments here so mandatory Plan Review and optional workflow review nodes receive the same de-duped user-authored context without caller-specific formatting.
|
||||
*
|
||||
* FNXC:AgentSteering 2026-06-30-13:18:
|
||||
* Reviewer callers must request an uncapped selection because older user requirements remain binding quality-gate context; the default limit stays for non-review prompts that need bounded context.
|
||||
*/
|
||||
export function selectUserCommentsForAgentContext(
|
||||
task: { comments?: TaskComment[] },
|
||||
opts: { limit?: number } = {},
|
||||
task: { comments?: TaskComment[]; steeringComments?: SteeringComment[] },
|
||||
opts: { limit?: number | null } = {},
|
||||
): TaskComment[] {
|
||||
const limit = opts.limit ?? DEFAULT_USER_COMMENT_LIMIT;
|
||||
if (!task.comments || task.comments.length === 0 || limit <= 0) return [];
|
||||
const limit = opts.limit === null ? null : opts.limit ?? DEFAULT_USER_COMMENT_LIMIT;
|
||||
if (limit !== null && limit <= 0) return [];
|
||||
|
||||
const byId = new Map<string, TaskComment>();
|
||||
for (const comment of task.comments) {
|
||||
const candidates: TaskComment[] = [
|
||||
...(task.comments ?? []),
|
||||
...(task.steeringComments ?? []).map(normalizeSteeringComment),
|
||||
];
|
||||
|
||||
for (const comment of candidates) {
|
||||
if (comment.author !== "user") continue;
|
||||
byId.set(comment.id, comment);
|
||||
const existing = byId.get(comment.id);
|
||||
if (!existing || timestampMs(existing) <= timestampMs(comment)) {
|
||||
byId.set(comment.id, comment);
|
||||
}
|
||||
}
|
||||
|
||||
return [...byId.values()]
|
||||
.sort((a, b) => timestampMs(a) - timestampMs(b))
|
||||
.slice(-limit);
|
||||
const sorted = [...byId.values()].sort((a, b) => timestampMs(a) - timestampMs(b));
|
||||
return limit === null ? sorted : sorted.slice(-limit);
|
||||
}
|
||||
|
||||
export function buildUserCommentsPromptSection(
|
||||
|
||||
@@ -83,7 +83,7 @@ import { buildSessionSkillContext } from "./session-skill-context.js";
|
||||
import type { SkillSelectionContext } from "./skill-resolver.js";
|
||||
import { resolveMcpServersForStore } from "./mcp-resolution.js";
|
||||
import { reviewStep, type ReviewVerdict, type ReviewResult } from "./reviewer.js";
|
||||
import { selectUserCommentsForAgentContext } from "./agent-user-comments.js";
|
||||
import { buildUserCommentsPromptSection, selectUserCommentsForAgentContext } from "./agent-user-comments.js";
|
||||
import { resolveSandboxBackend } from "./sandbox/index.js";
|
||||
import type { SandboxBackend } from "./sandbox/types.js";
|
||||
import { ModelRegistry, SessionManager, type ToolDefinition, type AgentSession } from "@earendil-works/pi-coding-agent";
|
||||
@@ -6255,10 +6255,18 @@ export class TaskExecutor {
|
||||
const reviewCwd = resolveReviewCheckoutCwd(detail, worktreePath);
|
||||
const stepName = detail.steps[stepIndex]?.name ?? `Step ${stepIndex}`;
|
||||
const promptContent = detail.prompt ?? "";
|
||||
const userComments = selectUserCommentsForAgentContext(detail, { limit: null });
|
||||
// Merge per-task effective workflow settings (U3, KTD-3) so the validator
|
||||
// model-lane reads below pick up workflow values. Behavior-inert by default.
|
||||
const settings = await mergeEffectiveSettings(this.store, detail, await this.store.getSettings());
|
||||
|
||||
/*
|
||||
FNXC:AgentSteering 2026-06-30-12:37:
|
||||
Workflow graph step-review nodes are optional or mandatory reviewer gates. Pass canonical user comments and legacy steering into each per-cwd reviewer so workspace aggregation never drops operator requirements.
|
||||
|
||||
FNXC:AgentSteering 2026-06-30-13:20:
|
||||
Graph reviewer gates request uncapped comment context because every user-authored requirement can affect approval, including older steering retained on long-running tasks.
|
||||
*/
|
||||
const sem = this.options.semaphore;
|
||||
// FNXC:Workspace 2026-06-22-00:30: KTD3 — step-inversion review seam loops per sub-repo.
|
||||
// `reviewStep` stays single-cwd; THIS CALLER loops. Single-cwd by default reviews
|
||||
@@ -6295,6 +6303,7 @@ export class TaskExecutor {
|
||||
store: this.store,
|
||||
taskId: seamTask.id,
|
||||
task: detail,
|
||||
userComments: userComments.length > 0 ? userComments : undefined,
|
||||
agentPrompts: settings.agentPrompts,
|
||||
agentStore: this.options.agentStore,
|
||||
rootDir: this.rootDir,
|
||||
@@ -12794,7 +12803,14 @@ export class TaskExecutor {
|
||||
// validator model-lane reads below pick up workflow values; this tool
|
||||
// closure re-fetches independently. Behavior-inert by default.
|
||||
const latestDetailForReview = await store.getTask(taskId);
|
||||
const userComments = selectUserCommentsForAgentContext(latestDetailForReview);
|
||||
/*
|
||||
FNXC:AgentSteering 2026-06-30-12:38:
|
||||
The in-session fn_review_step tool must select fresh unified comments plus legacy steering immediately before spawning a reviewer so explicit user requirements posted during execution are reviewed.
|
||||
|
||||
FNXC:AgentSteering 2026-06-30-13:20:
|
||||
In-session reviewer calls use uncapped selection so the manual review tool cannot lose older operator instructions while validating the current step.
|
||||
*/
|
||||
const userComments = selectUserCommentsForAgentContext(latestDetailForReview, { limit: null });
|
||||
const settings = await mergeEffectiveSettings(store, latestDetailForReview, await store.getSettings());
|
||||
const reviewCwd = resolveReviewCheckoutCwd(latestDetailForReview, worktreePath);
|
||||
// Run the reviewer via semaphore.runNested so its slot accounting
|
||||
@@ -14028,6 +14044,15 @@ CRITICAL SCOPING RULES — read before doing anything else:
|
||||
- If NONE of the files in the diff scope are relevant to your review category (e.g. a UX/design reviewer with no UI/CSS/component files in scope, a security reviewer with no auth/network code in scope, an a11y reviewer with no markup changes), respond IMMEDIATELY with a single short approval line such as "No relevant changes in scope — approved." and STOP. Do not start exploring the codebase.
|
||||
- Your wall-clock budget is short. Spending it browsing unmodified files will cause this step to time out and block merge.`;
|
||||
|
||||
const latestTaskForUserComments = await this.store.getTask(task.id).catch(() => task);
|
||||
const workflowStepUserComments = selectUserCommentsForAgentContext(latestTaskForUserComments, { limit: null });
|
||||
const workflowStepUserCommentSection = buildUserCommentsPromptSection(workflowStepUserComments);
|
||||
|
||||
/*
|
||||
* FNXC:AgentSteering 2026-06-30-14:08:
|
||||
* Prompt/custom workflow-step reviewers, including Browser Verification agents, do not call reviewStep. They still gate quality, so their system prompt must carry the same canonical uncapped user comments plus legacy steering selected from a fresh task snapshot.
|
||||
*/
|
||||
|
||||
// (KTD-6) Verdict-contract reconciliation. The trailing-verdict JSON is the
|
||||
// gate-parsing contract — it only matters for steps that gate merge. A skill
|
||||
// step that isn't a gate (e.g. ce-plan / ce-work / ce-compound) produces
|
||||
@@ -14072,7 +14097,7 @@ Task Context:
|
||||
- Task Description: ${task.description}
|
||||
- Worktree: ${worktreePath}
|
||||
|
||||
${scopeBlock}
|
||||
${scopeBlock}${workflowStepUserCommentSection ? `\n\n${workflowStepUserCommentSection}` : ""}
|
||||
|
||||
Your role:
|
||||
- Execute this workflow step exactly as scoped.
|
||||
|
||||
@@ -480,7 +480,7 @@ export async function reviewStep(
|
||||
|
||||
reviewText = "";
|
||||
const reducedRequest = buildReducedReviewRequest(
|
||||
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline,
|
||||
taskId, stepNumber, stepName, reviewType, promptContent, cwd, baseline, options.userComments,
|
||||
);
|
||||
|
||||
try {
|
||||
@@ -703,7 +703,13 @@ function buildReducedReviewRequest(
|
||||
promptContent: string,
|
||||
cwd: string,
|
||||
baseline?: string,
|
||||
userComments?: TaskComment[],
|
||||
): string {
|
||||
/*
|
||||
FNXC:AgentSteering 2026-06-30-17:09:
|
||||
Context-limit retries may compact PROMPT.md, but reviewer gates still must evaluate every explicit user requirement.
|
||||
Preserve the canonical user comments and legacy steering section on reduced prompts so mandatory and optional reviews do not approve work that ignored operator feedback.
|
||||
*/
|
||||
return buildReviewRequest(
|
||||
taskId,
|
||||
stepNumber,
|
||||
@@ -712,7 +718,7 @@ function buildReducedReviewRequest(
|
||||
buildReducedTaskPromptSummary(promptContent),
|
||||
cwd,
|
||||
baseline,
|
||||
undefined,
|
||||
userComments,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -141,6 +141,7 @@ import { createRunAuditor, generateSyntheticRunId } from "./run-audit.js";
|
||||
import { resolveAndEmitGoalContext } from "./goal-injection-diagnostics.js";
|
||||
import { accumulateSessionTokenUsage } from "./session-token-usage.js";
|
||||
import { reviewStep } from "./reviewer.js";
|
||||
import { selectUserCommentsForAgentContext } from "./agent-user-comments.js";
|
||||
|
||||
|
||||
export interface TriageProcessorOptions {
|
||||
@@ -1963,6 +1964,20 @@ export class TriageProcessor {
|
||||
}
|
||||
}
|
||||
|
||||
const latestTaskForReview = await this.store.getTask(task.id).catch((error: unknown) => {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
planLog.warn(`${task.id}: failed to load fresh task comments for Plan Review; using supplied task snapshot: ${message}`);
|
||||
return task;
|
||||
});
|
||||
const userComments = selectUserCommentsForAgentContext(latestTaskForReview, { limit: null });
|
||||
|
||||
/*
|
||||
FNXC:AgentSteering 2026-06-30-12:33:
|
||||
Mandatory Plan Review must see user-authored unified comments and legacy steering from the latest task snapshot because it can block execution based on explicit operator requirements.
|
||||
|
||||
FNXC:AgentSteering 2026-06-30-13:19:
|
||||
Plan Review receives the uncapped reviewer context so older task comments cannot be silently dropped before the mandatory execution gate evaluates operator requirements.
|
||||
*/
|
||||
const review = await reviewStep(
|
||||
this.rootDir,
|
||||
task.id,
|
||||
@@ -1974,9 +1989,10 @@ export class TriageProcessor {
|
||||
{
|
||||
store: this.store,
|
||||
taskId: task.id,
|
||||
taskTitle: task.title,
|
||||
taskTitle: latestTaskForReview.title ?? task.title,
|
||||
settings,
|
||||
task,
|
||||
task: latestTaskForReview,
|
||||
userComments: userComments.length > 0 ? userComments : undefined,
|
||||
rootDir: this.rootDir,
|
||||
agentStore: this.options.agentStore,
|
||||
pluginRunner: this.options.pluginRunner,
|
||||
|
||||
Reference in New Issue
Block a user