fix(FN-WF): stop the journal announcing aborts that never happened, and assert it
The operator journal is a deliverable. Nothing asserted it, and three defects lived there. ABORT BREADCRUMB. `awaitAbortInFlightTaskWork` wrote `Pause abort marked` before inspecting any surface, so a card with no session still announced an interruption: every newly created task logged `provenance=hard-cancel` a second after creation, because creation moves the card out of the planning lane and that move is user-sourced. Nothing was interrupted and the operator withdrew nothing — false on both counts, and the second time this label has lied. The in-memory marker is still claimed synchronously, before any await, because the graph-failure classifiers depend on it; only the operator-facing line waits for evidence. DUPLICATE APPROVAL. Landing requires TWO consecutive clean approvals of the same candidate. Both wrote the identical sentence with the identical SHA, so a safety feature read as a duplicated invocation and was reported as an anomaly. The line now carries its pass number. DEAD RECOVERY. That same line is a contract: SelfHealingManager parses it with `/AI merge review \(pass \d+\): approved …/` to recover approved SHAs. No emitter ever wrote the parenthetical, so the parser matched nothing, `hasApprovedAiMergeReview` always answered false, and the recovery it guards could not run. Two sides individually reasonable, coupled through a log line nobody compared — the same shape as every other defect in this series. Emitter and parser now agree, and a test pins them against each other so a one-sided edit fails instead of silently killing the path again. COVERAGE. New pipeline-smoke scenario S20 drives a task to merge on all three coding built-ins and asserts the journal an operator actually reads: no abort claimed on an uninterrupted card, no line written twice in a row, no approval whose own text says it verified nothing. It reproduced the duplicate deterministically on its first run, which is the point — every anomaly reported this week was plainly visible in that journal and invisible to this lane. pnpm lint 0 errors, test:gate green, engine typecheck clean, pipeline-smoke 93/93.
This commit is contained in:
7
.changeset/journal-anomalies-and-scenarios.md
Normal file
7
.changeset/journal-anomalies-and-scenarios.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: The task journal no longer announces aborts that never happened or repeats the merge approval twice.
|
||||
category: fix
|
||||
dev: Three journal defects and the coverage gap that hid them. (1) `awaitAbortInFlightTaskWork` wrote its `Pause abort marked` breadcrumb before inspecting any surface, so every newly created task announced an interruption seconds after creation — creation moves the card out of the planning lane and that move is user-sourced, producing a `hard-cancel` label on a card nobody withdrew. The in-memory marker is still claimed synchronously (the graph-failure classifiers depend on it, and it must precede any await); only the operator-facing line now waits for evidence. (2) Landing requires two consecutive clean approvals of the same candidate, and both wrote the identical sentence, so a safety feature read as a duplicated invocation; the line now carries its pass number. (3) That same line is a contract: `SelfHealingManager.getApprovedAiMergeReviewShas` parses it with `/AI merge review \(pass \d+\): approved …/`, a parenthetical no emitter ever wrote, so `hasApprovedAiMergeReview` always answered false and the recovery it guards was dead. Emitter and parser now agree and are pinned against each other. New pipeline-smoke scenario S20 asserts the journal itself across all three coding built-ins — no abort claimed on an uninterrupted card, no line written twice in a row, no approval that records it verified nothing — and reproduced the duplicate deterministically on the first run.
|
||||
@@ -78,6 +78,16 @@ function logText(store: ReturnType<typeof createMockStore>): string {
|
||||
return store.logEntry.mock.calls.map((call: unknown[]) => call[1]).join("\n");
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:PausedAbortProvenance 2026-08-26-09:52:
|
||||
The operator-facing breadcrumb now waits until a surface was genuinely aborted, so a test that asserts
|
||||
the LABEL must give the abort something to abort. That is also the honest fixture: these cases are
|
||||
about what an operator is told when work is interrupted, and nothing was being interrupted here.
|
||||
*/
|
||||
function withActiveSession(executor: TaskExecutor, taskId: string): void {
|
||||
(executor as any).activeSessions.set(taskId, { session: { dispose: () => {} } });
|
||||
}
|
||||
|
||||
/** Every write the executor made to the row, so `userPaused` can be asserted negatively. */
|
||||
function updatePatches(store: ReturnType<typeof createMockStore>): Record<string, unknown>[] {
|
||||
return store.updateTask.mock.calls.map((call: unknown[]) => (call[1] ?? {}) as Record<string, unknown>);
|
||||
@@ -96,6 +106,7 @@ describe("pause-abort provenance truthfulness (KB-PROV)", () => {
|
||||
describe("awaitAbortInFlightTaskWork derives provenance from userCanceled", () => {
|
||||
it("labels an engine-initiated abort `engine-abort`, never `hard-cancel`", async () => {
|
||||
const { store, task, executor } = makeExecutor();
|
||||
withActiveSession(executor, task.id);
|
||||
|
||||
await executor.awaitAbortInFlightTaskWork(task.id, "parent moved from in-progress to todo");
|
||||
|
||||
@@ -107,8 +118,31 @@ describe("pause-abort provenance truthfulness (KB-PROV)", () => {
|
||||
expect((executor as any).userCanceledTaskIds.has(task.id)).toBe(false);
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:PausedAbortProvenance 2026-08-26-09:52:
|
||||
THE REPORTED DEFECT: the breadcrumb was written before any surface was inspected, so a card with no
|
||||
session at all still announced an interruption. Every newly created task logged
|
||||
`Pause abort marked: provenance=hard-cancel` a second or two after creation — creation moves the
|
||||
card out of the planning lane, and that move is user-sourced. Nothing was interrupted and the
|
||||
operator withdrew nothing; the line was false on both counts. The in-memory marker still records,
|
||||
because the graph-failure classifiers depend on it and it must be claimed before any await.
|
||||
*/
|
||||
it("says nothing to the operator when there was nothing to abort", async () => {
|
||||
const { store, task, executor } = makeExecutor();
|
||||
|
||||
await executor.awaitAbortInFlightTaskWork(task.id, "task moved out of planning to todo", {
|
||||
userCanceled: true,
|
||||
});
|
||||
|
||||
expect(logText(store)).not.toContain("Pause abort marked");
|
||||
expect(logText(store)).not.toContain("Pause abort cleanup completed");
|
||||
// The marker itself is still recorded for the classifiers that consume it.
|
||||
expect(provenanceOf(executor, task.id)).toBe("hard-cancel");
|
||||
});
|
||||
|
||||
it("keeps `hard-cancel` for an operator-canceled abort", async () => {
|
||||
const { store, task, executor } = makeExecutor();
|
||||
withActiveSession(executor, task.id);
|
||||
|
||||
await executor.awaitAbortInFlightTaskWork(task.id, "user moved task from in-progress to todo", {
|
||||
userCanceled: true,
|
||||
@@ -142,6 +176,7 @@ describe("pause-abort provenance truthfulness (KB-PROV)", () => {
|
||||
describe("surface enumeration: each abort caller gets a truthful label", () => {
|
||||
it("FN-8596 repro — an ENGINE-sourced in-progress -> todo move labels the abort `engine-abort`", async () => {
|
||||
const { store, task, executor } = makeExecutor();
|
||||
withActiveSession(executor, task.id);
|
||||
|
||||
await store._triggerAsync("task:moved", { task, from: "in-progress", to: "todo", source: "engine" });
|
||||
await (executor as any).pendingTaskDisposals.get(task.id);
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
/*
|
||||
FNXC:MergeReviewConfirmation 2026-08-26-10:11:
|
||||
The AI merge review's approval line is a CONTRACT, not a message: `SelfHealingManager` parses it back
|
||||
out of the task log to learn which squash SHAs were approved, and gates a recovery path on finding one.
|
||||
|
||||
Both sides were individually reasonable and had never been compared. The emitter wrote
|
||||
`AI merge review: approved squash <sha>`; the parser required `AI merge review (pass N): approved …`.
|
||||
So the parser matched nothing, `hasApprovedAiMergeReview` always answered false, and the recovery it
|
||||
guards could not run — a dead path, invisible, with no failing test anywhere.
|
||||
|
||||
The same line is also written TWICE per squash by design: landing requires two consecutive clean
|
||||
approvals of the same candidate. Word for word, that read to an operator as a duplicated invocation,
|
||||
and was reported as an anomaly. The pass number makes the safety feature legible.
|
||||
|
||||
These tests pin emitter and parser against each other. Either one drifting alone fails here.
|
||||
*/
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
/** The exact expression SelfHealingManager uses to recover approved SHAs from the journal. */
|
||||
const SELF_HEALING_APPROVAL_RE = /AI merge review \(pass \d+\): approved(?:\s+(?:squash|commit)\s+([0-9a-f]{7,40}))?/i;
|
||||
|
||||
const SHA = "784ab76e192b88f4c5764e893031e05396c19fdd";
|
||||
|
||||
/** Mirrors the emitter in merger-ai.ts; the source guard below proves it has not drifted. */
|
||||
function approvalLine(pass: number, options: { unconfirmed?: number } = {}): string {
|
||||
const suffixes = [
|
||||
options.unconfirmed ? `${options.unconfirmed} prior finding(s) unconfirmed` : "",
|
||||
pass >= 2 ? "confirmation pass" : "",
|
||||
].filter(Boolean);
|
||||
return `AI merge review (pass ${pass}): approved squash ${SHA}${suffixes.length ? ` — ${suffixes.join("; ")}` : ""}`;
|
||||
}
|
||||
|
||||
describe("AI merge review approval line contract", () => {
|
||||
it("is parseable by the self-healing recovery that consumes it", () => {
|
||||
const match = approvalLine(1).match(SELF_HEALING_APPROVAL_RE);
|
||||
expect(match, "the recovery path finds no approved SHA if this stops matching").not.toBeNull();
|
||||
expect(match?.[1]).toBe(SHA);
|
||||
});
|
||||
|
||||
it("keeps the SHA reachable when optional clauses are present", () => {
|
||||
// The capture requires the SHA immediately after `approved`; a clause inserted before it would
|
||||
// silently push the SHA out of reach and re-create the dead path.
|
||||
const match = approvalLine(2, { unconfirmed: 3 }).match(SELF_HEALING_APPROVAL_RE);
|
||||
expect(match?.[1]).toBe(SHA);
|
||||
});
|
||||
|
||||
/* Two clean approvals are required before landing; the operator must be able to tell them apart. */
|
||||
it("distinguishes the confirmation pass from the first approval", () => {
|
||||
const first = approvalLine(1);
|
||||
const second = approvalLine(2);
|
||||
|
||||
expect(first).not.toBe(second);
|
||||
expect(second).toContain("confirmation pass");
|
||||
expect(first).not.toContain("confirmation pass");
|
||||
for (const line of [first, second]) expect(line).toMatch(SELF_HEALING_APPROVAL_RE);
|
||||
});
|
||||
|
||||
/*
|
||||
Structural guard: the emitter and the parser live in different files, which is exactly how they
|
||||
drifted. Pin both literals so a one-sided edit fails here instead of silently killing the recovery.
|
||||
*/
|
||||
it("holds emitter and parser to the same shape", async () => {
|
||||
const { readFile } = await import("node:fs/promises");
|
||||
|
||||
const emitter = await readFile(new URL("../merge/merger-ai.ts", import.meta.url), "utf8");
|
||||
expect(emitter, "the emitter must write the parenthetical the parser requires")
|
||||
.toContain("AI merge review (pass ${approvalNumber}): approved squash ${candidateSha}");
|
||||
|
||||
const consumer = await readFile(new URL("../self-healing.ts", import.meta.url), "utf8");
|
||||
expect(consumer).toContain("AI merge review \\(pass \\d+\\): approved");
|
||||
});
|
||||
});
|
||||
@@ -98,6 +98,57 @@ export const PIPELINE_SCENARIO_DRIVERS = {
|
||||
}),
|
||||
s02Act: driver("run Planning through merge on the production graph", driveMerged),
|
||||
|
||||
/*
|
||||
FNXC:PipelineSmoke 2026-08-26-10:11:
|
||||
THE JOURNAL IS A DELIVERABLE. Every anomaly reported from a live board this week was visible in the
|
||||
task log and invisible to this lane, because no scenario asserted what an operator actually reads:
|
||||
an abort breadcrumb on a card that was never interrupted, the same line written twice, and an
|
||||
approval whose own text admitted it had verified nothing.
|
||||
|
||||
Those are not cosmetic. Each one is the observable trace of a real defect — a lying provenance
|
||||
label, a duplicated invocation, and a merge approved without checks — and each was dismissed as log
|
||||
noise until it was traced. Asserting the journal is how this lane catches that class at all.
|
||||
|
||||
The checks are deliberately about SHAPE, never wording: a specific sentence would pin prose and
|
||||
break on the first honest rewording.
|
||||
*/
|
||||
s20Arrange: driver("create a task whose operator journal must stay clean", async (context) => {
|
||||
await arrangeTask(context, "S20");
|
||||
}),
|
||||
s20Act: driver("drive to merge, then assert the journal an operator reads has no anomalies", async (context) => {
|
||||
const task = taskFor(context);
|
||||
context.result = await context.harness.driveToDeclaredTerminal(task.id, "merged-done");
|
||||
|
||||
const live = await context.harness.freshTask(task.id);
|
||||
const entries = (live.log ?? []).map((entry) => `${entry.action ?? ""}`.trim()).filter(Boolean);
|
||||
const violations: string[] = [];
|
||||
|
||||
/* A card that completed normally was never interrupted, so it must not claim it was. */
|
||||
const abortClaims = entries.filter((line) => line.startsWith("Pause abort marked"));
|
||||
if (abortClaims.length > 0) {
|
||||
violations.push(`journal claims an abort on an uninterrupted card: ${abortClaims.join(" | ")}`);
|
||||
}
|
||||
|
||||
/* The same sentence twice in a row is a duplicated invocation wearing a log line. */
|
||||
for (let index = 1; index < entries.length; index += 1) {
|
||||
if (entries[index] === entries[index - 1]) {
|
||||
violations.push(`journal repeats a line back to back: ${entries[index]}`);
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
/* An approval that says it could not check is the failure mode this whole series was about. */
|
||||
const unverifiedApproval = entries.find((line) =>
|
||||
/approved|approve\b/i.test(line) && /could not run|unavailable|nothing was verified|not verified/i.test(line));
|
||||
if (unverifiedApproval) {
|
||||
violations.push(`journal records an approval that verified nothing: ${unverifiedApproval}`);
|
||||
}
|
||||
|
||||
if (violations.length > 0) {
|
||||
throw new Error(`S20 operator journal anomalies:\n- ${violations.join("\n- ")}`);
|
||||
}
|
||||
}),
|
||||
|
||||
s03Arrange: driver("create an unpromoted Idea", async (context) => {
|
||||
await arrangeTask(context, "S03", { initialColumn: "creation" });
|
||||
}),
|
||||
|
||||
@@ -231,11 +231,34 @@ export const PIPELINE_SCENARIOS: readonly PipelineScenario[] = [
|
||||
act: PIPELINE_SCENARIO_DRIVERS.s19Act,
|
||||
invariants: ["no legacy column id appears in persisted trail", "both cloned built-ins converge"],
|
||||
},
|
||||
/*
|
||||
FNXC:PipelineSmoke 2026-08-26-10:11:
|
||||
The journal an operator reads is a deliverable, and until now nothing asserted it. Every anomaly
|
||||
reported from a live board this week was plainly visible there and invisible to this lane: an abort
|
||||
breadcrumb on a card that was never interrupted, a line written twice, and an approval whose own
|
||||
text admitted it had verified nothing. Each was the observable trace of a real defect — a lying
|
||||
provenance label, a duplicated invocation, a merge approved without checks — and each was dismissed
|
||||
as noise until traced. Running on all three coding built-ins because the defects were not specific
|
||||
to one lane.
|
||||
*/
|
||||
{
|
||||
id: "S20",
|
||||
title: "A completed task leaves a journal with no anomalies",
|
||||
workflows: ["builtin:coding-ideas", "builtin:coding-ideas-v2", "builtin:coding"],
|
||||
expectedTerminal: "merged-done",
|
||||
arrange: PIPELINE_SCENARIO_DRIVERS.s20Arrange,
|
||||
act: PIPELINE_SCENARIO_DRIVERS.s20Act,
|
||||
invariants: [
|
||||
"no abort is claimed on an uninterrupted card",
|
||||
"no journal line is written twice in a row",
|
||||
"no approval records that it verified nothing",
|
||||
],
|
||||
},
|
||||
] as const;
|
||||
|
||||
export function assertPipelineScenarioTable(scenarios: readonly PipelineScenario[] = PIPELINE_SCENARIOS): void {
|
||||
if (scenarios.length !== 19) {
|
||||
throw new Error(`Pipeline smoke requires exactly 19 scenarios; received ${scenarios.length}.`);
|
||||
if (scenarios.length !== 20) {
|
||||
throw new Error(`Pipeline smoke requires exactly 20 scenarios; received ${scenarios.length}.`);
|
||||
}
|
||||
const ids = new Set(scenarios.map((scenario) => scenario.id));
|
||||
if (ids.size !== scenarios.length || [...ids].some((id, index) => id !== `S${String(index + 1).padStart(2, "0")}`)) {
|
||||
|
||||
@@ -14,7 +14,7 @@ const describeIfReady = hasGit ? pgDescribe : describe.skip;
|
||||
|
||||
// Structural validation stays outside the PostgreSQL lifecycle so it cannot start a runtime.
|
||||
describe("pipeline smoke scenario contract", () => {
|
||||
it("keeps the executable scenario manifest closed at nineteen entries", () => {
|
||||
it("keeps the executable scenario manifest closed at twenty entries", () => {
|
||||
assertPipelineScenarioTable();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -51,7 +51,7 @@ describeIfReady("pipeline smoke: resilience scenarios", () => {
|
||||
});
|
||||
afterAll(pg.afterAll);
|
||||
|
||||
it.each(executableVariants(["S17", "S18", "S19"]))(
|
||||
it.each(executableVariants(["S17", "S18", "S19", "S20"]))(
|
||||
"$scenario.id runs $workflowId $variant",
|
||||
async ({ scenario: selected, workflowId, variant }) => {
|
||||
const context = { harness, workflowId, variant };
|
||||
|
||||
@@ -15,7 +15,7 @@ import type { PausedAbortProvenance } from "./paused-abort-provenance.js";
|
||||
|
||||
export type AwaitAbortInFlightTaskWorkDeps = {
|
||||
userCanceledTaskIds: Set<string>;
|
||||
markPausedAborted: (taskId: string, provenance: PausedAbortProvenance, source: string) => void;
|
||||
markPausedAborted: (taskId: string, provenance: PausedAbortProvenance, source: string, options?: { quiet?: boolean }) => void;
|
||||
untrackStuckTask: (taskId: string) => void;
|
||||
clearWorkflowRerunWatchdog: (taskId: string) => void;
|
||||
clearCompletedTaskWatchdog: (taskId: string) => void;
|
||||
@@ -55,7 +55,17 @@ export async function awaitAbortInFlightTaskWork(
|
||||
FNXC:WorkflowLifecycle 2026-07-26-11:20:
|
||||
KB-PROV: Stamp the provenance the caller actually reported instead of a blanket `hard-cancel`. `options.userCanceled` is already the truthful operator-intent signal every caller computes (`source === "user"`, soft-delete, the registered move disposer), so derive the label from it: operator withdrawal keeps `hard-cancel`, everything else is an `engine-abort`. Without this, the FN-8596 engine rerun bounce told the operator `provenance=hard-cancel` for work the engine itself re-dispatched, and any future consumer branching on `hard-cancel` would read an engine bounce as an operator withdrawal. Behaviour is unchanged: `userPaused` is still never set by engine rebounds, and the downstream classifiers accept both labels via `isGenericAbortProvenance()`.
|
||||
*/
|
||||
deps.markPausedAborted(taskId, options.userCanceled ? "hard-cancel" : "engine-abort", `abort-in-flight:${reason}`);
|
||||
/*
|
||||
FNXC:PausedAbortProvenance 2026-08-26-09:52:
|
||||
Record the marker now (it must be claimed synchronously, before any await, so two concurrent
|
||||
disposals cannot race) but hold the operator-facing breadcrumb until we know a surface was really
|
||||
aborted. Emitted eagerly, it announced an interruption on cards that had no session at all: every
|
||||
newly created task logged `Pause abort marked: provenance=hard-cancel` seconds after creation,
|
||||
because creation moves the card out of the planning lane and that move is user-sourced.
|
||||
*/
|
||||
const abortProvenance = options.userCanceled ? "hard-cancel" : "engine-abort";
|
||||
const abortSource = `abort-in-flight:${reason}`;
|
||||
deps.markPausedAborted(taskId, abortProvenance, abortSource, { quiet: true });
|
||||
deps.untrackStuckTask(taskId);
|
||||
deps.clearWorkflowRerunWatchdog(taskId);
|
||||
deps.clearCompletedTaskWatchdog(taskId);
|
||||
@@ -177,6 +187,12 @@ export async function awaitAbortInFlightTaskWork(
|
||||
deps.stuckAborted.delete(taskId);
|
||||
|
||||
if (hadActiveSurface) {
|
||||
/*
|
||||
The deferred breadcrumb: something was genuinely interrupted, so say so and why. Emitted directly
|
||||
rather than through a second `markPausedAborted` call, whose first-mark deduplication would
|
||||
suppress it — the marker was already recorded above, quietly.
|
||||
*/
|
||||
deps.safeLogEntry(taskId, `Pause abort marked: provenance=${abortProvenance} source=${abortSource}`);
|
||||
executorLog.log(`${taskId}: awaited abort of in-flight work — ${reason}`);
|
||||
deps.safeLogEntry(
|
||||
taskId,
|
||||
|
||||
@@ -1369,8 +1369,9 @@ export function buildPauseAbortMarkerDeps(host: any): any {
|
||||
...facadeFields(host, [
|
||||
"pausedAborted", "pausedAbortProvenance", "completionFinalizedTaskIds",
|
||||
]),
|
||||
markPausedAborted: (id: string, provenance?: unknown, source?: string) =>
|
||||
host.markPausedAborted(id, provenance, source),
|
||||
/* FNXC:PausedAbortProvenance 2026-08-26-09:52: forward the quiet flag so the breadcrumb can wait for evidence. */
|
||||
markPausedAborted: (id: string, provenance?: unknown, source?: string, options?: { quiet?: boolean }) =>
|
||||
host.markPausedAborted(id, provenance, source, options),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -14,17 +14,34 @@ export type MarkPausedAbortedDeps = {
|
||||
safeLogEntry: (taskId: string, message: string) => void;
|
||||
};
|
||||
|
||||
/*
|
||||
FNXC:PausedAbortProvenance 2026-08-26-09:52:
|
||||
`quiet` records the marker WITHOUT the task-card breadcrumb, for a caller that does not yet know
|
||||
whether anything was actually aborted.
|
||||
|
||||
The breadcrumb exists so an operator can see why a workflow was interrupted. It was emitted before
|
||||
any surface was inspected, so a card with no session at all still announced one: every newly created
|
||||
task logged `Pause abort marked: provenance=hard-cancel` 1-2 seconds after creation, because creation
|
||||
moves the card out of the planning lane and that move is user-sourced. Nothing was interrupted and
|
||||
the operator withdrew nothing — the line was false on both counts, and it is the second time this
|
||||
label has lied (see the KB-PROV note in paused-abort-provenance.ts).
|
||||
|
||||
The in-memory marker itself is still recorded unconditionally: it is claimed synchronously before any
|
||||
await precisely so two concurrent disposals cannot race, and the graph-failure classifiers depend on
|
||||
it. Only the operator-facing line waits for evidence.
|
||||
*/
|
||||
export function markPausedAborted(
|
||||
deps: MarkPausedAbortedDeps,
|
||||
taskId: string,
|
||||
provenance: PausedAbortProvenance = "hard-cancel",
|
||||
source = "unspecified",
|
||||
options: { quiet?: boolean } = {},
|
||||
): void {
|
||||
const previousProvenance = deps.pausedAbortProvenance.get(taskId);
|
||||
const alreadyMarked = deps.pausedAborted.has(taskId);
|
||||
deps.pausedAborted.add(taskId);
|
||||
deps.pausedAbortProvenance.set(taskId, provenance);
|
||||
if (!alreadyMarked || previousProvenance !== provenance) {
|
||||
if (options.quiet !== true && (!alreadyMarked || previousProvenance !== provenance)) {
|
||||
deps.safeLogEntry(
|
||||
taskId,
|
||||
`Pause abort marked: provenance=${provenance} source=${source}${previousProvenance && previousProvenance !== provenance ? ` previous=${previousProvenance}` : ""}`,
|
||||
|
||||
@@ -3285,9 +3285,33 @@ async function mergeAndReview(input: {
|
||||
};
|
||||
await persistState(persistedState, state);
|
||||
if (clean) {
|
||||
/*
|
||||
FNXC:MergeReviewConfirmation 2026-08-26-10:11:
|
||||
Landing requires TWO consecutive clean approvals of the same candidate (see the
|
||||
`consecutiveCleanApprovals >= 2` gate below), so this line is written twice per squash — by
|
||||
design, and previously WORD FOR WORD. An operator reading the task journal saw the same
|
||||
sentence repeated with the same SHA and had no way to tell a confirmation from a duplicated
|
||||
invocation; it was reported as an anomaly precisely because the log made a safety feature
|
||||
look like a bug. Number the approval so the second one reads as what it is.
|
||||
*/
|
||||
const approvalNumber = state.consecutiveCleanApprovals;
|
||||
/*
|
||||
The SHA sits immediately after `approved`, and the pass number inside `(pass N)`, because
|
||||
`SelfHealingManager.getApprovedAiMergeReviewShas` PARSES this line with
|
||||
`/AI merge review \(pass \d+\): approved(?:\s+(?:squash|commit)\s+([0-9a-f]{7,40}))?/`.
|
||||
That parser has never matched anything: no emitter ever wrote the parenthetical, so
|
||||
`hasApprovedAiMergeReview` always answered false and the recovery it guards could not run.
|
||||
Two sides, each individually reasonable, coupled through a log line nobody compared — the
|
||||
same shape as every other defect in this series. Keep the suffixes AFTER the SHA so the
|
||||
capture group cannot be pushed out of reach by an optional clause.
|
||||
*/
|
||||
const suffixes = [
|
||||
unconfirmed ? `${unconfirmed} prior finding(s) unconfirmed` : "",
|
||||
approvalNumber >= 2 ? "confirmation pass" : "",
|
||||
].filter(Boolean);
|
||||
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}`);
|
||||
: `AI merge review (pass ${approvalNumber}): approved squash ${candidateSha}${suffixes.length ? ` — ${suffixes.join("; ")}` : ""}`);
|
||||
if (state.consecutiveCleanApprovals >= 2) {
|
||||
await assertCurrentEpisodeIdentity();
|
||||
return { squashSha: candidateSha === tipSha ? null : candidateSha, priorReasons: [] };
|
||||
|
||||
Reference in New Issue
Block a user