From 7d54e86e801987230990c249c9e66373261f14cd Mon Sep 17 00:00:00 2001 From: Fusion Agent Date: Sat, 22 Aug 2026 22:32:59 +0000 Subject: [PATCH] FN-159: filter AI merge protocol markers from blocking findings Prevent the AI merge reviewer from persisting its own protocol syntax as blocking review findings. - Register and filter all review protocol markers and disposition lines. - Aggregate reviewer prose into bounded findings and preserve reconciliation safety. - Release prior findings that are not re-confirmed on approved candidates. - Add reconciliation state, tests, documentation, and a patch changeset. Files changed: .../fn-159-ai-merge-protocol-marker-findings.md | 7 ++ docs/architecture.md | 2 +- packages/core/src/types/task/task-core.ts | 6 ++ .../engine/src/__tests__/merger-ai-prompts.test.ts | 39 ++++++++++ .../src/__tests__/merger-ai-squash-gates.test.ts | 6 ++ .../engine/src/__tests__/merger-ai.test.ts | 58 +++++++++++++++ .../engine/src/__tests__/workspace-merger.test.ts | 3 +- packages/engine/src/merge/merger-ai-prompts.ts | 53 +++++++++++--- packages/engine/src/merge/merger-ai.ts | 84 ++++++++++++++++++---- 9 files changed, 231 insertions(+), 27 deletions(-) Fusion-Task-Id: FN-159 Fusion-Task-Lineage: 6eea1dec-1252-4b8d-b227-c84c24ab60af Co-authored-by: Fusion --- ...n-159-ai-merge-protocol-marker-findings.md | 7 ++ docs/architecture.md | 2 +- packages/core/src/types/task/task-core.ts | 6 ++ .../src/__tests__/merger-ai-prompts.test.ts | 39 +++++++++ .../__tests__/merger-ai-squash-gates.test.ts | 6 ++ .../engine/src/__tests__/merger-ai.test.ts | 58 +++++++++++++ .../src/__tests__/workspace-merger.test.ts | 3 +- .../engine/src/merge/merger-ai-prompts.ts | 53 +++++++++--- packages/engine/src/merge/merger-ai.ts | 84 +++++++++++++++---- 9 files changed, 231 insertions(+), 27 deletions(-) create mode 100644 .changeset/fn-159-ai-merge-protocol-marker-findings.md diff --git a/.changeset/fn-159-ai-merge-protocol-marker-findings.md b/.changeset/fn-159-ai-merge-protocol-marker-findings.md new file mode 100644 index 0000000000..248ba071d6 --- /dev/null +++ b/.changeset/fn-159-ai-merge-protocol-marker-findings.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Stop AI merge from blocking on its own review protocol markers. +category: fix +dev: Adds a protocol marker registry, aggregates reviewer prose, and converges unreconfirmed approvals. diff --git a/docs/architecture.md b/docs/architecture.md index b3e17ae11e..72d20043a0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -732,7 +732,7 @@ Runtime action-gate flow (v1): - FN-8405 supplies the prerequisite declaration/resolution seam: `Task.declaredSymbols` is the durable source and `TaskStore.resolveTaskSymbolsForWorkItem({ taskId })` resolves through the owning task without reading its prompt. Scheduler admission does not acquire or admit locks here; FN-8306 is the separate consumer. - Batch 1 maintenance now includes one `fts-maintenance` step for both search indexes. The live `tasks_fts` branch still runs `merge` every tick, `optimize` every 4th tick, and `rebuild` above `32 MiB` or `1 MiB × live task count`. The archive `archived_tasks_fts` branch is lighter because archive writes are mostly append-only: `merge` every 8th tick, `optimize` every 24th tick, and `rebuild` above `64 MiB` or `512 KiB × archived row count`. Each branch is independently guarded by `fts5Available` and emits `task:fts-maintenance` run-audit telemetry with distinct `target` values (`tasks_fts` vs `archived_tasks_fts`). - AI merge clean-room worktrees are created under the configured worktrees directory's hidden container, `/.ai-merge/`, as `fusion-ai-merge-fn--` detached worktrees. See [orphan merge-body durable-write fences](solutions/reliability/merge-orphan-body-durable-write-fences.md) for the bounded abort/settle-latch investigation and its pinned write-frontier ratchet. When that container is repo-local, its relative path is added to the repo's local git exclude when possible (alongside the legacy `.fusion/ai-merge/` entry) so an in-flight clean room does not dirty the integration checkout. After `git worktree add` and before the merge/review loop, `runAiMerge` bootstraps the clean room with the shared merge dependency-sync helper: a configured `worktreeInitCommand` is authoritative and always runs, while unset settings infer `pnpm`/`npm`/`yarn`/`bun` installs from lockfiles and can skip only when the `node_modules/.fusion-install-marker` hash still matches. Failures and aborts hard-stop the AI merge before merge agents or verification run, and `merge:ai-deps-sync` records the command, skip state, and duration. Inline cleanup runs from `runAiMerge`'s clean-room `finally` for successful lands, empty/no-op finalization, concurrent-advance retries, and thrown/aborted merges. Cleanup canonicalizes the path, attempts `git worktree remove --force`, then uses a bounded errno-based filesystem retry (`EBUSY`, permission, descriptor, and non-empty failures); on Windows permission failures it clears read-only attributes before retrying. It always retains its existing `git worktree prune` follow-up: prune reclaims a registered-but-missing path, while a residually present path deliberately retains its registration so the next reaper can target it. Present-but-unregistered leftovers are removed through the same filesystem retry. Cleanup emits `merge:ai-worktree-cleanup` audit events for git-remove, fs-rm, and prune phases; benign already-absent/de-registered paths are idempotent success, while residual failures record `success: false`, `attempts`, `residual: true`, and `registrationRetained: true`. The native worktree backend shares that retry fallback only after its established recoverable Git-error classification; a stale `is not a working tree` registration still rethrows without filesystem fallback or prune. - - **AI review blocks are terminal by type:** `AiMergeBlockedError` is parked as `status:"failed"` before direct-merge recovery examines error text. Reviewer findings can naturally use words such as “conflict”; they must never enter git-conflict retry, bounce, or cooldown admission. The clean-room reviewer carries a current, normalized contract of at most eight unresolved squash-fidelity findings, retaining prior findings unless it explicitly acknowledges them under `RESOLVED_PRIOR_FINDINGS:`. The latest bounded rejection log is the crash-recovery source and contracts older than seven days expire deterministically. Blocking review is limited to branch preservation, squash collateral, and conflict soundness; whole-tree semantic concerns outside the squash footprint are advisory/follow-up material because the merger is not authorized to edit them. + - **AI review blocks are terminal by type:** `AiMergeBlockedError` is parked as `status:"failed"` before direct-merge recovery examines error text. Reviewer findings can naturally use words such as “conflict”; they must never enter git-conflict retry, bounce, or cooldown admission. The clean-room reviewer carries a current, normalized contract of at most eight unresolved squash-fidelity findings. Its four protocol markers (`REVIEW_VERDICT:`, `SEVERITY:`, `RESOLVED_PRIOR_FINDINGS:`, and `PRIOR_FINDING_DISPOSITIONS:`), plus disposition body lines, are never reasons and cannot enter the durable finding corpus; contiguous reviewer prose is one finding. An approval releases a prior `still-present` finding it does not re-confirm for that candidate, so repeated approval lands or blocks only on a named reviewer objection. The latest bounded rejection log is the crash-recovery source and contracts older than seven days expire deterministically. Blocking review is limited to branch preservation, squash collateral, and conflict soundness; whole-tree semantic concerns outside the squash footprint are advisory/follow-up material because the merger is not authorized to edit them. - Worktrees-dir sweeps that list direct children of `` (pool idle scan, orphan cleanup/reap, self-healing unregistered-orphan reap, and cap enforcement) must exclude the `.ai-merge` container by name; those one-level sweeps never inspect or recycle clean rooms beneath it. Batch 1 sweeps stale AI merge clean-room worktrees under the new `/.ai-merge/` root and still scans legacy `.fusion/ai-merge/` plus legacy `tmpdir()` locations for pre-relocation leftovers; candidates are bounded to names starting with `fusion-ai-merge-`. `runAiMerge` registers each live clean-room worktree in `activeSessionRegistry` with kind `ai-merge` as soon as the directory exists and keeps both raw and canonical paths registered for the duration of the merge, so the dedicated periodic sweep and pre-merge prune defer when either path is active (including concurrent same-task merge attempts). The default age gate is 2 hours; task-aware cleanup uses a 10-minute grace period for `done`/`archived` tasks and for genuinely missing/deleted task rows, and every removal path is clamped by the same 10-minute minimum-age floor so a freshly created worktree is never reaped. Transient `getTask` lookup failures (for example PostgreSQL availability/transaction errors) are not treated as deletion evidence; they log a warning, emit `lookup-error` only if eventually removed, and retain the conservative 2-hour gate. The sweep canonicalizes paths before checking `activeSessionRegistry`, attempts `git worktree remove --force ` before bounded errno-based filesystem retry/removal (including Windows read-only clearing), runs its existing `git worktree prune` after cleanup attempts, and emits `worktree:tempdir-sweep` run-audit telemetry for removal attempts and failures including `attempts`, `residual`, and `registrationRetained` for an undeletable path. Fresh directories, active-session paths, and individual removal failures are skipped/logged without aborting the maintenance cycle. - `recoverGhostReviewTasks()` is a fallback only for idle, non-terminal `in-review` states. Terminal/actionable states (notably `status: "failed"`) are preserved and **not** auto-kicked back to `todo`. diff --git a/packages/core/src/types/task/task-core.ts b/packages/core/src/types/task/task-core.ts index 9671295c8a..74246f4a40 100644 --- a/packages/core/src/types/task/task-core.ts +++ b/packages/core/src/types/task/task-core.ts @@ -783,6 +783,12 @@ export interface AiMergeReviewReconciliation { findings: AiMergeReviewFinding[]; consecutiveCleanApprovals: number; correctivePasses: number; + /* + FNXC:AIMergeReviewReconciliation 2026-08-22-22:04: + FN-159 permits one same-candidate re-ask for malformed acknowledgements so reviewer protocol + defects cannot reset clean approvals forever; omit the field until that first re-ask. + */ + invalidAcknowledgementCandidateSha?: string; terminal?: boolean; } diff --git a/packages/engine/src/__tests__/merger-ai-prompts.test.ts b/packages/engine/src/__tests__/merger-ai-prompts.test.ts index e1974fbd21..208d06a57e 100644 --- a/packages/engine/src/__tests__/merger-ai-prompts.test.ts +++ b/packages/engine/src/__tests__/merger-ai-prompts.test.ts @@ -1,6 +1,9 @@ import { describe, expect, it } from "vitest"; +import * as mergerAiPrompts from "../merge/merger-ai-prompts.js"; + import { + PRIOR_FINDING_DISPOSITIONS_MARKER, REVIEW_VERDICT_MARKER, RESOLVED_PRIOR_FINDINGS_MARKER, buildMergeSystemPrompt, @@ -96,6 +99,33 @@ describe("merger-ai prompt/verdict re-exports", () => { }); }); + it("keeps prior-finding dispositions out of reject reasons while retaining their structure", () => { + const result = parseReviewVerdict([ + "The squash adds an unaccounted 30 ms async teardown delay to", + "`AddMarkerModal-info-service-runtime.spec.ts`.", + "", + PRIOR_FINDING_DISPOSITIONS_MARKER, + "finding-1-1: still-present", + `${REVIEW_VERDICT_MARKER} reject`, + ].join("\n")); + expect(result).toMatchObject({ verdict: "reject", severity: "blocking", priorFindingDispositions: [{ id: "finding-1-1", disposition: "still-present" }] }); + expect(result.reasons).toEqual(["The squash adds an unaccounted 30 ms async teardown delay to `AddMarkerModal-info-service-runtime.spec.ts`."]); + expect(result.reasons.join("\n")).not.toMatch(/PRIOR_FINDING_DISPOSITIONS|finding-1-1: still-present/); + }); + + it("keeps every exported protocol marker out of rejection prose", () => { + const exportedMarkers = Object.entries(mergerAiPrompts) + .filter(([name, value]): value is string => name.endsWith("_MARKER") && typeof value === "string") + .map(([, marker]) => marker); + + expect(exportedMarkers).not.toHaveLength(0); + for (const marker of exportedMarkers) { + expect(mergerAiPrompts.isAiMergeProtocolLine(`${marker} anything`)).toBe(true); + const result = parseReviewVerdict(`${marker} anything\n${REVIEW_VERDICT_MARKER} reject`); + expect(result.reasons.some((reason) => reason.includes(marker))).toBe(false); + } + }); + it("keeps non-negotiable clean-room and verdict-marker prompt content", () => { expect(buildMergeSystemPrompt()).toContain("## AI merge — clean room"); expect(buildMergeSystemPrompt()).toContain( @@ -143,6 +173,15 @@ describe("parseReviewVerdict — reason recovery (FN-8004)", () => { ); }); + it("groups contiguous prose into one readable finding", () => { + const result = parseReviewVerdict([ + "The squash adds an unaccounted 30 ms async teardown delay to", + "`AddMarkerModal-info-service-runtime.spec.ts`.", + `${REVIEW_VERDICT_MARKER} reject`, + ].join("\n")); + expect(result.reasons).toEqual(["The squash adds an unaccounted 30 ms async teardown delay to `AddMarkerModal-info-service-runtime.spec.ts`."]); + }); + it("orders recovered reasons nearest-the-verdict first (the closing argument)", () => { const result = parseReviewVerdict( [ diff --git a/packages/engine/src/__tests__/merger-ai-squash-gates.test.ts b/packages/engine/src/__tests__/merger-ai-squash-gates.test.ts index c370a03429..cea4cef6b0 100644 --- a/packages/engine/src/__tests__/merger-ai-squash-gates.test.ts +++ b/packages/engine/src/__tests__/merger-ai-squash-gates.test.ts @@ -46,6 +46,12 @@ function makeStore(scope: string[], overrides: Record = {}) { getSettings: vi.fn(async () => ({ merger: { mode: "ai", maxReviewPasses: 0 } })), parseFileScopeFromPrompt: vi.fn(async () => scope), updateTask: vi.fn(async (_id: string, patch: object) => Object.assign(task, patch)), + /* FNXC:MergerAiReview 2026-08-22-22:20: FN-159 reconciliation persists every candidate and verdict through the production atomic CAS seam, so this mutable store double must apply its callback patch to the same task returned by getTask. */ + updateTaskAtomic: vi.fn(async (_id: string, mutate: (current: typeof task) => Partial | undefined) => { + const patch = mutate(task); + if (patch) Object.assign(task, patch); + return task; + }), moveTask: vi.fn(async (_id: string, column: string) => Object.assign(task, { column })), appendAgentLog: vi.fn(async () => undefined), logEntry: vi.fn(async () => undefined), diff --git a/packages/engine/src/__tests__/merger-ai.test.ts b/packages/engine/src/__tests__/merger-ai.test.ts index 61c5ae48fd..eaf841d446 100644 --- a/packages/engine/src/__tests__/merger-ai.test.ts +++ b/packages/engine/src/__tests__/merger-ai.test.ts @@ -332,6 +332,64 @@ describe("runAiMerge", () => { expect(mergePrompts.slice(1).every((prompt) => prompt.includes("[finding-1-1] " + blocker))).toBe(true); expect(mergePrompts.slice(1).every((prompt) => !prompt.includes("reviewer reconciliation"))).toBe(true); }); + it("does not clear a re-confirmed blocker through a contradictory duplicate acknowledgement", async () => { + const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" }); + const blocker = "server pages still bypass the live authorization guard"; + const { store } = makeStore(dir, {}, { merger: { mode: "ai", maxReviewPasses: 1 } }); + let reviews = 0; + await expect(runAiMerge(store, dir, "FN-1", { manual: true }, { + mergeAgent: realMergeAgent("fusion/fn-1"), + reviewAgent: async () => ++reviews === 1 + ? `${blocker}\nREVIEW_VERDICT: reject` + : "PRIOR_FINDING_DISPOSITIONS:\nfinding-1-1: still-present\nfinding-1-1: corrected\nREVIEW_VERDICT: approve", + })).rejects.toMatchObject({ reasons: [blocker] } satisfies Partial); + }); + + it("converges after repeated unusable acknowledgements on the same candidate", async () => { + const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" }); + const { store, logs } = makeStore(dir); + const result = await runAiMerge(store, dir, "FN-1", { manual: true }, { + mergeAgent: realMergeAgent("fusion/fn-1"), + reviewAgent: async () => "PRIOR_FINDING_DISPOSITIONS:\nunknown: still-present\nREVIEW_VERDICT: approve", + }); + expect(result.merged).toBe(true); + expect(logs.some((line) => /AI merge BLOCKED/.test(line))).toBe(false); + }); + + it("releases unreconfirmed blockers on approval and converges", async () => { + const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" }); + const { store } = makeStore(dir, {}, { merger: { mode: "ai", maxReviewPasses: 2 } }); + let reviews = 0; + const result = await runAiMerge(store, dir, "FN-1", { manual: true }, { + mergeAgent: realMergeAgent("fusion/fn-1"), + reviewAgent: async () => ++reviews === 1 + ? "actual squash fidelity defect\nREVIEW_VERDICT: reject" + : "REVIEW_VERDICT: approve", + }); + expect(result.merged).toBe(true); + const persisted = await store.getTask("FN-1"); + expect(persisted?.aiMergeReviewReconciliation).toBeNull(); + }); + + it("filters disposition protocol from a blocking rejection before corrective merge", async () => { + const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" }); + const { store, logs } = makeStore(dir, {}, { merger: { mode: "ai", maxReviewPasses: 2 } }); + const prompts: string[] = []; + let reviews = 0; + const result = await runAiMerge(store, dir, "FN-1", { manual: true }, { + mergeAgent: async (cwd, prompt) => { prompts.push(prompt); await realMergeAgent("fusion/fn-1")(cwd, prompt); }, + reviewAgent: async () => { + reviews++; + if (reviews === 1) return ["The squash adds an unaccounted 30 ms async teardown delay to", "`AddMarkerModal-info-service-runtime.spec.ts`.", "", "PRIOR_FINDING_DISPOSITIONS:", "finding-1-1: still-present", "REVIEW_VERDICT: reject"].join("\n"); + return "REVIEW_VERDICT: approve"; + }, + }); + expect(result.merged).toBe(true); + expect(prompts[1]).toContain("The squash adds an unaccounted 30 ms async teardown delay to `AddMarkerModal-info-service-runtime.spec.ts`."); + expect(prompts[1]).not.toMatch(/PRIOR_FINDING_DISPOSITIONS|finding-1-1: still-present/); + expect(logs.some((line) => /AI merge BLOCKED/.test(line))).toBe(false); + }); + it("merges a clean branch, advances main, and finalizes the task", async () => { const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" }); const { store, emitted } = makeStore(dir); diff --git a/packages/engine/src/__tests__/workspace-merger.test.ts b/packages/engine/src/__tests__/workspace-merger.test.ts index cc58e3348a..250a4027d2 100644 --- a/packages/engine/src/__tests__/workspace-merger.test.ts +++ b/packages/engine/src/__tests__/workspace-merger.test.ts @@ -298,7 +298,7 @@ describeIfGit("landWorkspaceTask — per-repo merge loop (Phase C U1)", () => { reviewPrompts.push(prompt); reviews++; return reviews === 1 - ? `${finding}\nSEVERITY: blocking\nREVIEW_VERDICT: reject` + ? `${finding}\n${PRIOR_FINDING_DISPOSITIONS_MARKER}\nfinding-1-1: still-present\nSEVERITY: blocking\nREVIEW_VERDICT: reject` : `${RESOLVED_PRIOR_FINDINGS_MARKER} ${finding}\n${PRIOR_FINDING_DISPOSITIONS_MARKER}\nfinding-1-1: corrected\nREVIEW_VERDICT: approve`; }, }); @@ -307,6 +307,7 @@ describeIfGit("landWorkspaceTask — per-repo merge loop (Phase C U1)", () => { /* FNXC:WorkspaceMergeTests 2026-08-20-23:23: FN-090 requires two clean confirmations after a corrected finding, so the final approval pass is intentionally a second independent reviewer session. */ expect(reviewPrompts).toHaveLength(3); expect(reviewPrompts[1]).toContain(finding); + expect(reviewPrompts[1]).not.toContain("finding-1-1: still-present"); }); it("per-repo resolution: each repo lands on its OWN origin/HEAD branch (override-stripping)", async () => { diff --git a/packages/engine/src/merge/merger-ai-prompts.ts b/packages/engine/src/merge/merger-ai-prompts.ts index 40bf13ce23..ac6ebb25d3 100644 --- a/packages/engine/src/merge/merger-ai-prompts.ts +++ b/packages/engine/src/merge/merger-ai-prompts.ts @@ -26,8 +26,17 @@ export interface AiMergeReviewVerdict { } export const REVIEW_VERDICT_MARKER = "REVIEW_VERDICT:"; +export const SEVERITY_MARKER = "SEVERITY:"; export const RESOLVED_PRIOR_FINDINGS_MARKER = "RESOLVED_PRIOR_FINDINGS:"; export const PRIOR_FINDING_DISPOSITIONS_MARKER = "PRIOR_FINDING_DISPOSITIONS:"; +/* +FNXC:MergerAiReview 2026-08-22-22:04: +FN-090 added the fourth protocol marker without teaching reason recovery about it, letting +Fusion persist its own acknowledgement syntax as blocking feedback. Keep every marker in this +registry so a future fifth marker is rejected by construction at both parser and durable seams. +*/ +export const AI_MERGE_PROTOCOL_MARKERS = [REVIEW_VERDICT_MARKER, SEVERITY_MARKER, RESOLVED_PRIOR_FINDINGS_MARKER, PRIOR_FINDING_DISPOSITIONS_MARKER] as const; +const DISPOSITION_BODY_RE = /^[A-Za-z0-9_-]+\s*:\s*(corrected|absent-from-squash|still-present)$/i; const VERDICT_LINE_RE = /REVIEW_VERDICT:\s*(approve|reject)\b/i; const SEVERITY_LINE_RE = /SEVERITY:\s*(blocking|advisory)\b/i; const RESOLVED_PRIOR_FINDINGS_LINE_RE = /RESOLVED_PRIOR_FINDINGS:\s*(.*)$/i; @@ -39,6 +48,7 @@ free-form analysis can run long; the corrective re-merge prompt only needs the c and an unbounded splice would paste an entire transcript into it. */ const MAX_RECOVERED_PRECEDING_REASONS = 8; +export const MAX_REASON_CHARS = 500; /** * Parse the reviewer's free-form output. Fail-safe: no/garbled output, or a @@ -111,7 +121,7 @@ function extractResolvedPriorReasons(lines: string[]): string[] { const resolved = inline && !/^none\b/i.test(inline) ? [inline] : []; for (let index = markerIndex + 1; index < lines.length; index++) { const line = lines[index]; - if (VERDICT_LINE_RE.test(line) || SEVERITY_LINE_RE.test(line) || !line.trim()) break; + if (VERDICT_LINE_RE.test(line) || SEVERITY_LINE_RE.test(line) || line.trim().toLowerCase().startsWith(PRIOR_FINDING_DISPOSITIONS_MARKER.toLowerCase()) || !line.trim()) break; if (!/^\s*(?:[-*•]|\d+[.)])\s+/.test(line)) return []; resolved.push(cleanReasonLine(line)); } @@ -125,7 +135,7 @@ function extractPriorFindingDispositions(lines: string[]): Array<{ id: string; d const result: Array<{ id: string; disposition: "corrected" | "absent-from-squash" | "still-present" }> = []; for (let i = index + 1; i < lines.length; i++) { const line = lines[i]; - if (!line.trim() || VERDICT_LINE_RE.test(line) || SEVERITY_LINE_RE.test(line)) break; + if (!line.trim() || VERDICT_LINE_RE.test(line) || SEVERITY_LINE_RE.test(line) || line.trim().toLowerCase().startsWith(RESOLVED_PRIOR_FINDINGS_MARKER.toLowerCase())) break; const match = cleanReasonLine(line).match(/^([A-Za-z0-9_-]+)\s*:\s*(corrected|absent-from-squash|still-present)\s*$/i); if (match) result.push({ id: match[1], disposition: match[2].toLowerCase() as "corrected" | "absent-from-squash" | "still-present" }); } @@ -140,13 +150,22 @@ function normalizeReasonIdentity(reason: string): string { return reason.trim().toLocaleLowerCase().replace(/[\s\p{P}]+/gu, " ").trim(); } +/** Whether a line is Fusion review protocol rather than reviewer reasoning. */ +export function isAiMergeProtocolLine(text: string): boolean { + const clean = cleanReasonLine(text); + return AI_MERGE_PROTOCOL_MARKERS.some((marker) => clean.toLowerCase().includes(marker.toLowerCase())) || DISPOSITION_BODY_RE.test(clean); +} + /** Lines that carry no reviewer reasoning and must never be reported as a reason. */ -function isNonReasonLine(line: string): boolean { +export function isNonReasonLine(line: string): boolean { const t = line.trim(); if (!t) return true; if (SEVERITY_LINE_RE.test(t)) return true; if (VERDICT_LINE_RE.test(t)) return true; if (RESOLVED_PRIOR_FINDINGS_LINE_RE.test(t)) return true; + /* FNXC:MergerAiReview 2026-08-22-22:26: Protocol markers at line start stay non-reasons even with malformed values; inline marker prose is left to the durable-corpus defence-in-depth boundary. */ + if (AI_MERGE_PROTOCOL_MARKERS.some((marker) => t.toLowerCase().startsWith(marker.toLowerCase()))) return true; + if (DISPOSITION_BODY_RE.test(cleanReasonLine(t))) return true; // Markdown scaffolding the reviewer may emit around its analysis. if (/^#{1,6}\s/.test(t)) return true; if (/^(?:-{3,}|={3,}|`{3,})/.test(t)) return true; @@ -156,20 +175,34 @@ function isNonReasonLine(line: string): boolean { /* FNXC:MergerAiReview 2026-07-15-14:45: FN-8004 corrective merges must receive reviewer conclusions, never pasted diff or tool output. Track Markdown fences while recovering reasons on either side of a verdict so nearby evidence cannot displace actionable feedback. + +FNXC:MergerAiReview 2026-08-22-22:04: +FN-159 groups contiguous review prose because treating every line as a finding split one actionable +multi-line objection into truncated fragments that no corrective merge could address reliably. */ function collectReasonLines(lines: string[], start: number, end: number, step: 1 | -1): string[] { const reasons: string[] = []; + let group: string[] = []; let inFence = false; + const flush = () => { + if (!group.length) return; + const ordered = step === -1 ? group.reverse() : group; + const reason = ordered.join(" "); + reasons.push(reason.length > MAX_REASON_CHARS ? `${reason.slice(0, MAX_REASON_CHARS - 1)}…` : reason); + group = []; + }; for (let i = start; step === 1 ? i < end : i >= end; i += step) { - if (/^\s*(?:`{3,}|~{3,})/.test(lines[i])) { - inFence = !inFence; - continue; - } - if (inFence || isNonReasonLine(lines[i])) continue; - reasons.push(cleanReasonLine(lines[i])); + const line = lines[i]; + const bullet = /^\s*(?:[-*•]|\d+[.)])\s+/.test(line); + if (/^\s*(?:`{3,}|~{3,})/.test(line)) { flush(); inFence = !inFence; continue; } + if (inFence || isNonReasonLine(line)) { flush(); continue; } + if (bullet && step === 1) flush(); + group.push(cleanReasonLine(line)); + if (bullet && step === -1) flush(); if (reasons.length >= MAX_RECOVERED_PRECEDING_REASONS) break; } - return reasons; + flush(); + return reasons.slice(0, MAX_RECOVERED_PRECEDING_REASONS); } /* diff --git a/packages/engine/src/merge/merger-ai.ts b/packages/engine/src/merge/merger-ai.ts index a7d9d14a8b..7e21a09ad3 100644 --- a/packages/engine/src/merge/merger-ai.ts +++ b/packages/engine/src/merge/merger-ai.ts @@ -135,6 +135,7 @@ import { buildReviewSystemPrompt, buildStashResolvePrompt, buildStashResolveSystemPrompt, + isAiMergeProtocolLine, parseReviewVerdict, } from "./merger-ai-prompts.js"; @@ -229,7 +230,7 @@ function boundBlockingReviewReasons(reasons: readonly string[]): string[] { for (const reason of reasons) { const display = reason.trim().replace(/\s+/g, " "); const key = normalizeBlockingReviewReason(display); - if (!key || seen.has(key)) continue; + if (!key || isAiMergeProtocolLine(display) || seen.has(key)) continue; seen.add(key); bounded.push(display); if (bounded.length === MAX_BLOCKING_REVIEW_REASONS) break; @@ -428,14 +429,18 @@ async function ensureCommitTaskMetadata( // --------------------------------------------------------------------------- export { + AI_MERGE_PROTOCOL_MARKERS, + PRIOR_FINDING_DISPOSITIONS_MARKER, REVIEW_VERDICT_MARKER, RESOLVED_PRIOR_FINDINGS_MARKER, + SEVERITY_MARKER, buildMergePrompt, buildMergeSystemPrompt, buildReviewPrompt, buildReviewSystemPrompt, buildStashResolvePrompt, buildStashResolveSystemPrompt, + isAiMergeProtocolLine, parseReviewVerdict, } from "./merger-ai-prompts.js"; export type { AiMergeReviewSeverity, AiMergeReviewVerdict } from "./merger-ai-prompts.js"; @@ -2951,7 +2956,7 @@ async function mergeAndReview(input: { await persistState(persistedState, state); needsMerge = false; } - const candidateSha = state.candidateSha!; + const candidateSha: string = state.candidateSha!; await assertCurrentEpisodeIdentity(); await setStatus("reviewing"); const diffStat = await git(["diff", "--stat", `${tipSha}..${candidateSha}`], mergeRoot); @@ -2961,28 +2966,77 @@ async function mergeAndReview(input: { // A review response can arrive after an operator dismisses its finding or pushes a new source. await assertCurrentEpisodeIdentity(); const ids = new Set(state.findings.map((finding) => finding.id)); - const seen = new Set(); - const invalidAcknowledgement = (verdict.priorFindingDispositions ?? []).some(({ id }) => !ids.has(id) || seen.has(id) || !seen.add(id)); - const dispositions = new Map((verdict.priorFindingDispositions ?? []).map((entry) => [entry.id, entry.disposition])); - const findings: NonNullable["findings"] = state.findings.map((finding) => { + const dispositionCounts = new Map(); + for (const { id } of verdict.priorFindingDispositions ?? []) { + dispositionCounts.set(id, (dispositionCounts.get(id) ?? 0) + 1); + } + const invalidAcknowledgement = [...dispositionCounts].some(([id, count]) => !ids.has(id) || count > 1); + /* FNXC:MergerAiReview 2026-08-22-22:26: Unknown and duplicate acknowledgements are unusable; a contradictory duplicate must never clear a real blocker. */ + const dispositions = new Map((verdict.priorFindingDispositions ?? []) + .filter((entry) => ids.has(entry.id) && dispositionCounts.get(entry.id) === 1) + .map((entry) => [entry.id, entry.disposition])); + let findings: NonNullable["findings"] = state.findings.map((finding) => { const disposition = dispositions.get(finding.id); return disposition === "corrected" || disposition === "absent-from-squash" ? { ...finding, disposition } : disposition === "still-present" ? { ...finding, disposition } : finding; }); - const newFindings = verdict.verdict === "reject" && verdict.severity !== "advisory" ? boundBlockingReviewReasons(verdict.reasons).map((text, index) => ({ id: `finding-${state!.correctivePasses + 1}-${index + 1}`, text, disposition: "still-present" as const })) : []; + /* + FNXC:MergerAiReview 2026-08-22-22:04: + FN-159 filters protocol at durable finding construction as defence in depth: R1 recognizes + today's markers, while this independent boundary prevents a future marker from polluting the + reconciliation corpus as FN-090 did after FN-062. + */ + const recoveredReasons = boundBlockingReviewReasons(verdict.reasons); + const newFindings = verdict.verdict === "reject" && verdict.severity !== "advisory" + ? (recoveredReasons.length ? recoveredReasons : ["reviewer rejected the merge without a stated reason"]) + .map((text, index) => ({ id: `finding-${state!.correctivePasses + 1}-${index + 1}`, text, disposition: "still-present" as const })) + : []; + if (verdict.verdict === "approve") { + /* FNXC:MergerAiReview 2026-08-22-22:26: A malformed duplicate that says still-present still retains the real blocker. */ + const reConfirmed = new Set((verdict.priorFindingDispositions ?? []) + .filter((entry) => ids.has(entry.id) && entry.disposition === "still-present") + .map((entry) => entry.id)); + const released = findings.filter((finding) => finding.disposition === "still-present" && !reConfirmed.has(finding.id)); + if (released.length) { + const at = new Date().toISOString(); + findings = findings.map((finding) => released.some(({ id }) => id === finding.id) + ? { ...finding, disposition: "absent-from-squash", audit: [...(finding.audit ?? []), { at, actor: "ai-merge-review", disposition: "absent-from-squash", reason: `not re-confirmed on approved candidate ${candidateSha}` }] } + : finding); + await log(`AI merge review: approved; released unreconfirmed finding(s): ${released.map(({ id }) => id).join(", ")}`); + } + } state = { ...state, findings: [...findings, ...newFindings] }; const stillPresent: NonNullable["findings"] = state.findings.filter((finding) => finding.disposition === "still-present"); - const clean = verdict.verdict === "approve" && !invalidAcknowledgement && stillPresent.length === 0; + const repeatedInvalidAcknowledgement: boolean = invalidAcknowledgement && state.invalidAcknowledgementCandidateSha === candidateSha; + const unusableAcknowledgement: boolean = invalidAcknowledgement && !repeatedInvalidAcknowledgement; + const clean: boolean = verdict.verdict === "approve" && !unusableAcknowledgement && stillPresent.length === 0; await audit.git({ type: "merge:ai-review-verdict", target: integrationBranch, metadata: { taskId, verdict: verdict.verdict, severity: verdict.severity, squashSha: candidateSha } }); if (verdict.verdict === "approve") { const unconfirmed = state.findings.filter((finding) => finding.disposition === "pending").length; - state = { ...state, consecutiveCleanApprovals: clean ? state.consecutiveCleanApprovals + 1 : 0 }; + state = { + ...state, + consecutiveCleanApprovals: clean ? state.consecutiveCleanApprovals + 1 : 0, + ...(unusableAcknowledgement ? { invalidAcknowledgementCandidateSha: candidateSha } : {}), + }; await persistState(persistedState, state); - await log(clean ? `AI merge review: approved${unconfirmed ? ` — ${unconfirmed} prior finding(s) unconfirmed` : ""} squash ${candidateSha}` : `AI merge review: approved — reconciliation acknowledgement invalid`); - if (clean && state.consecutiveCleanApprovals >= 2) { - await assertCurrentEpisodeIdentity(); - return { squashSha: candidateSha === tipSha ? null : candidateSha, priorReasons: [] }; + if (clean) { + await log(repeatedInvalidAcknowledgement + ? "AI merge review: approved; ignoring repeated unusable prior-finding acknowledgement" + : `AI merge review: approved${unconfirmed ? ` — ${unconfirmed} prior finding(s) unconfirmed` : ""} squash ${candidateSha}`); + if (state.consecutiveCleanApprovals >= 2) { + await assertCurrentEpisodeIdentity(); + return { squashSha: candidateSha === tipSha ? null : candidateSha, priorReasons: [] }; + } + continue; // Direct confirmation review of exactly the same candidate; no merge agent and no budget spend. } - if (clean) continue; // direct confirmation review of exactly the same candidate; no merge agent and no budget spend. + if (unusableAcknowledgement && stillPresent.length === 0) { + await log("AI merge review: approved; prior-finding acknowledgement unusable (unknown or duplicated id) — re-asking on the same candidate"); + continue; + } + if (repeatedInvalidAcknowledgement && stillPresent.length === 0) { + await log("AI merge review: approved; ignoring repeated unusable prior-finding acknowledgement"); + continue; + } + if (stillPresent.length) await log(`AI merge review: approved but ${stillPresent.length} finding(s) re-confirmed still-present — corrective pass`); } if (verdict.verdict === "reject" && verdict.severity === "advisory" && stillPresent.length === 0) { await log(`AI merge: landing with unresolved advisory concern(s): ${verdict.reasons.join("; ")}`); @@ -2990,7 +3044,7 @@ async function mergeAndReview(input: { } if (stillPresent.length === 0) { state = { ...state, terminal: true }; await persistState(persistedState, state); - throw new AiMergeBlockedError(taskId, ["review acknowledgement is malformed or non-actionable"]); + throw new AiMergeBlockedError(taskId, ["reviewer rejected the merge without a stated reason"]); } if (state.correctivePasses >= maxPasses) { state = { ...state, terminal: true }; await persistState(persistedState, state);