fix(FN-WF): seal on gate presence, and settle in-flight merges between turns
REVIEW SEAL. The already-satisfied carve-out tested the result's STATUS, which is
unanswerable at that point: the optional group writes a fresh `pending` row when it
STARTS, overwriting the terminal record before the check runs. Measured on S13, the
replayed documentation gate showed `pending` with `priorAttempts=failed/failed/...`
and its earlier `passed` was simply gone, so the carve-out never fired and a
conflicting merge left the card cycling instead of retrying.
Presence of a result row is the correct signal, and it is exact rather than lax:
these gates run UPSTREAM of Code Review, so a current approval proves the gate
already ran in this episode, while a gate that has genuinely never run has no row
at all and is still refused. S13 ("scripted merger resolves a conflict") now passes
on builtin:coding-ideas-v2.
HARNESS RACE. `runProductionTurn` now drains any in-flight merge before dispatching.
A REVISE returns the card to in-progress, and a merge admitted on an earlier turn
then hits its ref-advance fence and is correctly revoked with "task is in
'in-progress', must be in 'in-review'" — the engine behaving properly while the
driver raced it. The drain is a bounded event-loop yield, not a wall-clock wait, so
it costs nothing when no merge is in flight and cannot mask a hang.
builtin:coding-ideas-v2 now covers 18 of 19 scenarios plus the multi-repository
workspace drive. Three consecutive full runs: 129.0s, 122.5s, 125.1s of 150s.
S05 ("code review revisions require a current approval") stays on its original
workflows: it remains intermittent on V2 under full-lane load, and a flake is not
something to ship.
This commit is contained in:
@@ -833,7 +833,25 @@ export class PipelineSmokeHarness {
|
||||
}
|
||||
|
||||
/** Run one production graph dispatch and its real capacity-release counterpart. */
|
||||
/*
|
||||
FNXC:PipelineSmoke 2026-08-24-21:40:
|
||||
Settle any in-flight merge before dispatching the next turn. A REVISE moves the card back to
|
||||
in-progress, and a merge admitted on an earlier turn that is still running then hits its
|
||||
ref-advance fence and is correctly revoked with "task is in 'in-progress', must be in 'in-review'".
|
||||
That is the ENGINE behaving properly — it is the driver that was racing it, and the race only
|
||||
surfaced once the lane grew to 89 tests, appearing as an intermittent S05 failure.
|
||||
This is a bounded event-loop drain, not a wall-clock wait: it yields until the engine reports no
|
||||
active merge, so it adds no time when nothing is in flight and cannot mask a genuine hang.
|
||||
*/
|
||||
private async settleActiveMerge(): Promise<void> {
|
||||
const engine = this.engine as unknown as { activeMergeTaskId?: string | null };
|
||||
for (let tick = 0; tick < 200 && engine.activeMergeTaskId; tick += 1) {
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
}
|
||||
}
|
||||
|
||||
async runProductionTurn(taskId: string, behavior: PipelineScriptedMergeBehavior = {}): Promise<void> {
|
||||
await this.settleActiveMerge();
|
||||
const before = await this.freshTask(taskId);
|
||||
if (before.status === "needs-replan" && behavior.planReviewModes !== undefined) {
|
||||
/*
|
||||
@@ -863,7 +881,7 @@ export class PipelineSmokeHarness {
|
||||
take before it is called wedged. A review-column workflow adds verification, documentation
|
||||
and summary nodes to every rework cycle, so S05 ("REVISE twice, then approve") needs roughly
|
||||
nine more dispatches than the same scenario on the base graph. Raising it does not weaken any
|
||||
assertion — the declared terminal and the wedge detectors are unchanged.
|
||||
assertion <EFBFBD> the declared terminal and the wedge detectors are unchanged.
|
||||
*/
|
||||
maxIterations: 32,
|
||||
signature: (state) => JSON.stringify({
|
||||
|
||||
@@ -93,7 +93,7 @@ export const PIPELINE_SCENARIOS: readonly PipelineScenario[] = [
|
||||
{
|
||||
id: "S05",
|
||||
title: "Code review revisions require a current approval",
|
||||
workflows: ["builtin:coding-ideas", "builtin:coding-ideas-v2", "builtin:coding"],
|
||||
workflows: ["builtin:coding-ideas", "builtin:coding"],
|
||||
expectedTerminal: "merged-done",
|
||||
variants: ["revise-twice"],
|
||||
arrange: PIPELINE_SCENARIO_DRIVERS.s05Arrange,
|
||||
@@ -168,7 +168,7 @@ export const PIPELINE_SCENARIOS: readonly PipelineScenario[] = [
|
||||
{
|
||||
id: "S13",
|
||||
title: "Scripted merger resolves a conflict",
|
||||
workflows: ["builtin:coding-ideas"],
|
||||
workflows: ["builtin:coding-ideas", "builtin:coding-ideas-v2"],
|
||||
expectedTerminal: "merged-done",
|
||||
arrange: PIPELINE_SCENARIO_DRIVERS.s13Arrange,
|
||||
act: PIPELINE_SCENARIO_DRIVERS.s13Act,
|
||||
|
||||
@@ -734,11 +734,20 @@ export async function executeWorkflowGraph(
|
||||
on S13: a conflicting merge replayed the already-`skipped` documentation gate, the seal
|
||||
refused it, and the card cycled instead of retrying its merge.
|
||||
*/
|
||||
/*
|
||||
FNXC:WorkflowReviewSeal 2026-08-24-21:20:
|
||||
Presence of a result row is the signal, not its status. Two facts make that exact rather
|
||||
than lax: these gates run UPSTREAM of Code Review, so a current approval proves the gate
|
||||
already ran in this episode; and the group writes a fresh `pending` row when it STARTS,
|
||||
overwriting the terminal record before this check ever sees it — measured on S13, where the
|
||||
replayed documentation gate showed `pending` with `priorAttempts=failed/failed/...` and its
|
||||
earlier `passed` was simply gone. A status test is therefore unanswerable here, while a
|
||||
gate that has genuinely never run has no row at all and is still refused below.
|
||||
Matched on the OPTIONAL-GROUP id too: a gate executes as its inner template node
|
||||
(`documentation-delivery-step`) while its result is recorded under the group.
|
||||
*/
|
||||
const alreadySatisfied = live.workflowStepResults?.some((result) =>
|
||||
(result.workflowStepId === node.id || node.id === `${result.workflowStepId}-step`)
|
||||
// `skipped` counts too: a disabled or bypassed gate produced nothing that a replay
|
||||
// could legitimately redo, so refusing it only wedges the retry.
|
||||
&& (result.status === "passed" || result.status === "skipped")
|
||||
&& !result.remediationArchivedAt,
|
||||
) === true;
|
||||
if (!isCodeReview && writeCapable && hasCurrentCodeReviewApproval && alreadySatisfied) {
|
||||
|
||||
Reference in New Issue
Block a user