root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(engine): the merge-refusal reason was classified by a column id, and it lands in run-audit (#3098)

Browse Source 
Claimed `auto-merge-finalization.ts` — and this one is a **reversal of
an earlier audit in the same file**, which is the interesting part.

## The earlier note said "diagnostic only". It was wrong about the
consequence

`validateWorkflowDoneMergeProof` picks between two refusal reasons with
`task.column === "done"`. Both arms return `{ ok: false }`, so this
never changed which branch ran — and on that basis a prior pass recorded
it as *"REAL but DIAGNOSTIC-ONLY"* and declined it, reasoning that
widening a signature to improve an error string is a poor trade.

**The reason is not an error string.** It is written to run-audit
metadata alongside `previousColumn` — `merger-merge-lifecycle.test.ts`
asserts exactly that — and that row is what an operator reads to find
out why a merge was refused.

So on a board whose complete lane is not called `done`, a card resting
in that lane was refused with the generic `missing-merge-confirmation`:
the classification for a card that is **not in the complete lane at
all**. The audit trail recorded the opposite of what happened. A wrong
record is worse than a vague one, because it gets acted on.

## The trade was also cheaper than the note claimed

The function is **already async** and **already takes an options bag**.
`resolveFinalizationColumns`, two functions up in the same file,
**already builds this exact predicate** for its own guard.

Nothing new is resolved. The answer that existed is handed down instead
of being re-asked with an id — the half-conversion shape this program
keeps finding, here inside a single file, one line apart: the caller
guards on the resolved `isCompleteColumn(latest.column)`, then calls a
validator that re-asked the same question with the literal.

`isCompleteColumn` is **optional with the legacy literal as its
default** — the same default-to-legacy contract the lane-parameter
vocabulary uses elsewhere — and `check-lane-wiring` watches the
parameter, so the two call sites cannot silently stop passing it.

## Measured

- New `merge-proof-reason-renamed-complete-lane.test.ts` — **2 pass**.
- **MUTATION**: dropping the parameter fails the renamed case and leaves
the legacy **control** green. The control earns its place: a failure now
means *"renamed board"*, not *"the refusal stopped working"*.
- **Driven through `finalizeProvenAutoMergeTask`**, not by calling the
validator with the new argument. The contract under test is the
**wiring** — a test that passed the argument directly would assert my
own parameter works and prove nothing about the seam that was broken.
- **The audit row is asserted, not just the return value.** The return
value alone is not the contract that failed here.
- merger / auto-merge suites — **5 files / 159 tests pass**.
- `tsc --noEmit -p packages/engine` clean; census `--strict`,
`check-lane-wiring`, `check-inert-sync-lane-conversions`,
`check-fnxc-future-dates` clean.

## Census

`auto-merge-finalization.ts` stays at **2**, deliberately. Both
remaining entries are now documented **degraded-fallback arms** — the
resolver's `catch` and this parameter's default — which is the right
kind of literal rather than a missed conversion. Converting a fallback
to a resolution would defeat its purpose.

## A note on the FNXC gate

My first stamps were dated `2026-08-01` while local today is
`2026-07-31`. `check-fnxc-future-dates` caught it and I re-stamped.
Worth mentioning because it is the second time this session that a
date-only local-calendar comparison has caught a stamp written near
midnight — the gate is doing real work, not ceremony.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-31 04:22:33 -07:00
committed by GitHub
parent dca67a79a3
commit d448ab6951
2 changed files with 142 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

113
packages/engine/src/__tests__/merge-proof-reason-renamed-complete-lane.test.ts Normal file
View File

@@ -0,0 +1,113 @@
/*
FNXC:WorkflowResolvedColumns 2026-07-31-23:40:
THE MERGE-REFUSAL REASON WAS CLASSIFIED BY A COLUMN ID.
`validateWorkflowDoneMergeProof` picks between two refusal reasons with
`task.column === "done"`. Both arms return `{ ok: false }`, so this never changed WHICH branch ran —
which is why an earlier audit recorded it as "diagnostic only" and declined it.
That undersold it. The reason is not a log line: it is written to run-audit metadata alongside
`previousColumn`, and that record is what an operator reads to find out why a merge was refused. On a
board whose complete lane is not called `done`, a card sitting in that lane was refused with the
generic `missing-merge-confirmation` — the classification for a card that is NOT in the complete lane
at all. The audit trail said the opposite of what happened.
Driven through `finalizeProvenAutoMergeTask` rather than by calling the validator with the new
argument, because the contract this pins is the WIRING: the caller already resolves
`isCompleteColumn` for its own guard one line earlier, and the defect was that it did not hand that
answer down. A test that passed the argument directly would assert my own parameter works and prove
nothing about the seam.
*/
import { describe, expect, it, vi } from "vitest";
import type { MergeResult, Task, TaskStore, WorkflowIr } from "@fusion/core";
import { finalizeProvenAutoMergeTask } from "../auto-merge-finalization.js";
/** Complete lane is `shipped`; the board declares no column called `done`. */
const RENAMED_IR = {
version: "v2", id: "wf-renamed", name: "renamed", nodes: [], edges: [],
columns: [
{ id: "drafting", name: "Drafting", traits: [{ trait: "hold", config: { release: "capacity" } }] },
{ id: "building", name: "Building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
{ id: "checking", name: "Checking", traits: [{ trait: "merge" }] },
{ id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
],
} as unknown as WorkflowIr;
/** A card resting in a completion lane with NO durable merge proof — the refused case. */
function completedWithoutProof(column: string): Task {
return {
id: "FN-NOPROOF",
title: "landed without proof",
description: "t",
column,
dependencies: [],
steps: [{ status: "done" }],
currentStep: 0,
log: [],
createdAt: "2026-06-01T00:00:00.000Z",
updatedAt: "2026-06-01T00:00:00.000Z",
mergeDetails: { mergeConfirmed: false, noOpMerge: true },
} as unknown as Task;
}
function storeFor(task: Task, ir?: WorkflowIr) {
const recordRunAuditEvent = vi.fn(async () => undefined);
const store = {
getTask: vi.fn(async () => task),
updateTask: vi.fn(async () => task),
moveTask: vi.fn(async () => task),
logEntry: vi.fn(async () => undefined),
getSettings: vi.fn(async () => ({})),
recordRunAuditEvent,
/* Absent → the resolver's documented degraded fallback, i.e. the legacy `done` answer. */
...(ir
? {
getTaskWorkflowSelectionAsync: vi.fn(async () => ({ workflowId: "wf-renamed", stepIds: [] })),
getWorkflowDefinition: vi.fn(async () => ({ id: "wf-renamed", ir })),
}
: {}),
} as unknown as TaskStore;
return { store, recordRunAuditEvent };
}
async function finalize(task: Task, ir?: WorkflowIr) {
const { store, recordRunAuditEvent } = storeFor(task, ir);
const result = await finalizeProvenAutoMergeTask({
store,
taskId: task.id,
result: { task, ok: true, merged: true, noOp: true } as unknown as MergeResult,
source: "workflow-graph-merge-finalize",
});
return { result, recordRunAuditEvent };
}
describe("the merge-proof refusal reason names the board's own complete lane", () => {
/*
CONTROL. A default board answers `column === "done"` either way, so this passes with or without
the fix — it is here so a failure below means "renamed board", not "the refusal stopped working".
*/
it("classifies a proof-less card in the legacy `done` column (control)", async () => {
const { result } = await finalize(completedWithoutProof("done"));
expect(result).toEqual(expect.objectContaining({
outcome: "blocked", reason: "done-without-merge-confirmation",
}));
});
it("classifies a proof-less card in a RENAMED complete lane the same way", async () => {
const { result, recordRunAuditEvent } = await finalize(completedWithoutProof("shipped"), RENAMED_IR);
/* Against the literal this was `missing-merge-confirmation` — the classification for a card that
is not in the complete lane at all, which is the opposite of what happened. */
expect(result).toEqual(expect.objectContaining({
outcome: "blocked", reason: "done-without-merge-confirmation",
}));
/* The audit row is the artifact an operator actually reads; the return value alone is not the
contract that was broken. */
expect(recordRunAuditEvent).toHaveBeenCalledWith(expect.objectContaining({
metadata: expect.objectContaining({ previousColumn: "shipped", reason: "done-without-merge-confirmation" }),
}));
});
});

40
packages/engine/src/auto-merge-finalization.ts
View File

@@ -80,23 +80,40 @@ function hasIncompleteWorkflowSteps(task: Task): boolean {
export async function validateWorkflowDoneMergeProof(
task: Task,
options: { result?: MergeResult; checkWorkflowSteps?: boolean } = {},
options: {
result?: MergeResult;
checkWorkflowSteps?: boolean;
/*
FNXC:WorkflowResolvedColumns 2026-07-31-23:20:
The RESOLVED complete test, supplied by the caller. Omitted → the `done` literal, i.e. today's
behaviour, which is the same default-to-legacy contract the lane-parameter vocabulary uses
elsewhere. `resolveFinalizationColumns` in this file already builds exactly this predicate for
its own guard; the two callers below now hand it down instead of re-asking with an id.
*/
isCompleteColumn?: (columnId: string) => boolean;
} = {},
): Promise<WorkflowDoneMergeProofVerdict> {
const hasProof = hasDurableMergeProof(task, options.result);
/*
FNXC:WorkflowLifecycleColumns 2026-07-31-02:45 (audited — REAL but DIAGNOSTIC-ONLY):
FNXC:WorkflowResolvedColumns 2026-07-31-23:25 (the deferral is now paid — see the note above):
This literal selects which REASON STRING is reported, not which branch runs. Both arms return
`{ ok: false }`, so on a renamed board a card sitting in the complete lane is refused with the
`{ ok: false }`, so on a renamed board a card sitting in the complete lane was refused with the
generic `missing-merge-confirmation` instead of the specific `done-without-merge-confirmation`.
Worth recording rather than converting from here: the resolver two functions up already computes
`isCompleteColumn` for exactly this workflow, and threading it in is the right fix — but this
function does not receive it, and widening the signature to improve an error string is a change
whose cost outweighs the diagnosis it sharpens. The other two census entries in this file are NOT
defects: the `columnId === "done"` at the top is the resolver's documented degraded fallback (the
live arm calls `columnHasFlag`), and the `step.status` comparison is a STEP status, not a column.
The earlier note recorded this as "REAL but DIAGNOSTIC-ONLY" and declined it on the grounds that
widening a signature to improve an error string is a poor trade. That undersold the consequence:
this reason is not a log line. It is asserted as run-audit metadata alongside `previousColumn`
(`merger-merge-lifecycle.test.ts`), so the audit trail — the record an operator reads to find out
why a merge was refused — carried the wrong classification for every renamed board.
The trade is also cheaper than it looked. This function is ALREADY async and ALREADY takes an
options bag, and `resolveFinalizationColumns` two functions up ALREADY builds this exact predicate
for its own guard. Nothing new is resolved; the answer that existed is handed down instead of
being re-asked with an id — which is the half-conversion shape this program keeps finding, here
within one file.
*/
if (!hasProof) return { ok: false, reason: task.column === "done" ? "done-without-merge-confirmation" : "missing-merge-confirmation" };
const isCompleteLane = options.isCompleteColumn ? options.isCompleteColumn(task.column) : task.column === "done";
if (!hasProof) return { ok: false, reason: isCompleteLane ? "done-without-merge-confirmation" : "missing-merge-confirmation" };
if (options.checkWorkflowSteps !== false && hasIncompleteWorkflowSteps(task)) {
return { ok: false, reason: "incomplete-workflow-steps" };
}
@@ -210,7 +227,7 @@ export async function finalizeProvenAutoMergeTask({
* Workflow-owned completion requires current merge proof, not just a stale `mergeConfirmed` flag. A task cannot reach or remain accepted as `done` when workflow steps are still pending or a no-op claims landed files. Branch-only residue is ignored because squash landing validates the task patch, not branch-history cleanliness.
*/
if (isCompleteColumn(latest.column)) {
const proofVerdict = await validateWorkflowDoneMergeProof({ ...latest, mergeDetails: validationMergeDetails } as Task, { result });
const proofVerdict = await validateWorkflowDoneMergeProof({ ...latest, mergeDetails: validationMergeDetails } as Task, { result, isCompleteColumn });
if (!proofVerdict.ok) {
await recordFinalizationAudit({
store,
@@ -279,6 +296,7 @@ export async function finalizeProvenAutoMergeTask({
const proofVerdict = await validateWorkflowDoneMergeProof({ ...latest, mergeDetails } as Task, {
result,
checkWorkflowSteps: false,
isCompleteColumn,
});
if (!proofVerdict.ok) {
await recordFinalizationAudit({