fix(core): merge re-enqueue threw on every board with a renamed review column (#2819)
The single most consequential finding from the u12 seam-gate work, picked up because batch-core (#2783) merged without it and it is now unowned. ## The defect `enqueueMergeQueueInTransaction` gates on the task's column being a review column, and takes the board's review columns as an optional trailing argument. - `moves.ts:487` and `moves.ts:1153` — the automatic handoff-to-review path — resolve and pass them. - The public `enqueueMergeQueue` wrapper (`async-merge-coordination.ts:246`), reached through `store.enqueueMergeQueue`, **did not**, so it fell back to `new Set(["in-review"])`. This is not the quiet legacy-id degradation most of these seams produce. The reject branch records `mergeQueue:enqueue-rejected` and **throws** `MergeQueueInvalidColumnError`. Its production callers are `merger.ts:7251` and `self-healing.ts:10329` — so on any board whose review lane is renamed, the merge and recovery re-enqueue paths failed outright while the handoff path kept working. ## Measured, not asserted With the fix reverted, the renamed case fails with the exact predicted error and the controls stay green: ``` × renamed vocabulary: a task in the RENAMED review lane enqueues for merge MergeQueueInvalidColumnError: Task KB-001 is in column 'checking', not 'in-review'; cannot enqueue ✓ default vocabulary: a task in the review lane enqueues for merge ✓ renamed vocabulary: a task in the WIP lane is still REJECTED ✓ default vocabulary: a task in the WIP lane is still REJECTED Tests 1 failed | 3 passed (4) ``` With it: `Tests 4 passed (4)`. ## About the suite **Differential.** The fixture is the builtin coding workflow with only its column ids renamed, so the sole difference between the two runs is vocabulary — a hand-built graph would test the fixture's own transition table as much as the code. It asserts the rename actually landed (`checking` present, `in-review` absent), so a surviving literal cannot pass by luck, and it walks the graph rather than jumping, because moves are transition-validated. **Both negatives included.** A WIP-lane task must still be REJECTED under each vocabulary. Supplying the real columns must not degrade into "every column is a review column", which would let work merge straight out of the WIP lane — the failure mode a careless version of this fix would introduce. ## Why nothing caught it Partial supply. Two of three call sites passed the argument, so a check asking "does SOME caller supply this?" reported the seam as satisfied, and the lifecycle-column census counted the conversion as done. Closing that one-supplier floor in `scripts/check-inert-flag-seams.mjs` (on #2772) is what surfaced it. ## Verification - `pnpm test:gate` green - new suite 4/4; neighbouring merge-queue suites (`taskstore-lifecycle`, `store-in-review-stall`, `runtime-lifecycle-async`) 29/29 - `tsc -p packages/core` 0, lint 0 - changeset included (`@runfusion/fusion` patch) Note: #2772 still carries a TEMPORARY per-call-site exemption for this seam. Once this lands, that exemption's staleness check will fail and I will remove it there — it cannot outlive the fix. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
7
.changeset/merge-queue-renamed-review-column.md
Normal file
7
.changeset/merge-queue-renamed-review-column.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Fix merge re-enqueue failing on boards whose review column is renamed.
|
||||
category: fix
|
||||
dev: `enqueueMergeQueue` (and `store.enqueueMergeQueue`) now resolve the task's own review columns and forward them to `enqueueMergeQueueInTransaction`, which previously fell back to `new Set(["in-review"])` and threw `MergeQueueInvalidColumnError`. The two `moves.ts` handoff callers already supplied them; the merger and self-healing re-enqueue paths did not.
|
||||
@@ -0,0 +1,151 @@
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-14:35 (#2819 review — greptile, "renamed entries remain unleaseable"):
|
||||
|
||||
A QUEUED CARD ON A RENAMED BOARD MUST ACTUALLY BE LEASEABLE.
|
||||
|
||||
The first pass on this PR taught only the STALE SWEEP to resolve review lanes, so the row survived
|
||||
cleanup — and then both eligibility predicates in `acquireMergeQueueLease` still demanded the literal
|
||||
`in-review`, matched nothing, and the row sat in the queue forever. That is a worse failure than the
|
||||
one it replaced: a deleted row at least emits `mergeQueue:auto-cleanup-stale-row`, while an
|
||||
unleaseable row is a silent permanent stall with a healthy-looking queue.
|
||||
|
||||
WHY THE SWEEP FIX ALONE LOOKED GREEN. Its test asserted the row still EXISTS after acquisition. Row
|
||||
survival and row leaseability are different questions, and the sweep fix answered only the first —
|
||||
the acquire returning `null` was consistent with an empty queue and nothing distinguished them. So
|
||||
these cases assert the LEASE, which is the outcome the merger actually depends on.
|
||||
|
||||
BOTH ACQUIRE MODES ARE COVERED. Targeted acquire (`targetTaskId`) and queue-head acquire are separate
|
||||
code paths with separate predicates; the merger uses targeted, self-healing recovery uses the head.
|
||||
Converting one and leaving the other is the half-converted-pair shape this program keeps hitting.
|
||||
|
||||
WHY A LIVE STORE. The predicate is SQL evaluated by PostgreSQL against a column id that comes from
|
||||
the task's own persisted workflow. A mock cannot exercise it at all.
|
||||
|
||||
LANE. `.pg.test.ts`, skipped by `pgDescribe` when PostgreSQL is unreachable; throwaway per-file
|
||||
database; never port 4040.
|
||||
*/
|
||||
|
||||
import { it, expect, beforeAll, beforeEach, afterEach, afterAll } from "vitest";
|
||||
import {
|
||||
pgDescribe,
|
||||
createSharedPgTaskStoreTestHarness,
|
||||
type SharedPgTaskStoreHarness,
|
||||
} from "../../__test-utils__/pg-test-harness.js";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR } from "../../index.js";
|
||||
|
||||
pgDescribe("merge-queue lease acquisition under a renamed review lane", () => {
|
||||
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
|
||||
prefix: "fusion_mq_lease_renamed",
|
||||
});
|
||||
|
||||
beforeAll(h.beforeAll);
|
||||
beforeEach(h.beforeEach);
|
||||
afterEach(h.afterEach);
|
||||
afterAll(h.afterAll);
|
||||
|
||||
/**
|
||||
* The BUILTIN coding workflow with its column ids renamed and nothing else changed, so the only
|
||||
* difference between the control and the defect case is the vocabulary. `checking` collides with no
|
||||
* legacy literal, so a surviving `"in-review"` cannot pass by luck.
|
||||
*/
|
||||
async function seedRenamedWorkflow(): Promise<string> {
|
||||
const RENAME: Record<string, string> = {
|
||||
todo: "drafting",
|
||||
"in-progress": "building",
|
||||
"in-review": "checking",
|
||||
done: "shipped",
|
||||
};
|
||||
const rename = (id: string | undefined) => (id && RENAME[id]) ?? id;
|
||||
const ir = JSON.parse(JSON.stringify(BUILTIN_CODING_WORKFLOW_IR)) as {
|
||||
id: string;
|
||||
nodes?: { column?: string }[];
|
||||
columns?: { id: string }[];
|
||||
};
|
||||
ir.id = "custom:renamed-review-merge-queue-lease";
|
||||
for (const node of ir.nodes ?? []) node.column = rename(node.column);
|
||||
for (const column of ir.columns ?? []) column.id = rename(column.id) as string;
|
||||
|
||||
const ids = (ir.columns ?? []).map((column) => column.id);
|
||||
expect(ids).toContain("checking");
|
||||
expect(ids).not.toContain("in-review");
|
||||
|
||||
const created = await h.store().createWorkflowDefinition({
|
||||
name: "Renamed Review (merge queue lease)",
|
||||
kind: "workflow",
|
||||
ir,
|
||||
} as never);
|
||||
return (created as { id: string }).id;
|
||||
}
|
||||
|
||||
/** A card walked into its board's review lane and enqueued for merge. */
|
||||
async function seedQueuedTask(path: readonly string[], workflowId?: string): Promise<string> {
|
||||
const store = h.store();
|
||||
const task = await store.createTask({ title: "awaiting merge", description: "test", column: "todo" });
|
||||
if (workflowId) await store.writeTaskWorkflowSelection(task.id, workflowId, []);
|
||||
for (const step of path) await store.moveTask(task.id, step as never);
|
||||
store.taskCache.delete(task.id);
|
||||
|
||||
/* Prove the fixture: an unqueued card would fail to lease for a reason unrelated to vocabulary. */
|
||||
const entry = await store.enqueueMergeQueue(task.id);
|
||||
expect(entry.taskId).toBe(task.id);
|
||||
return task.id;
|
||||
}
|
||||
|
||||
/* Control. Passes before and after the fix, so a generally broken acquire cannot hide below. */
|
||||
it("default vocabulary: a queued card can be leased by the queue head", async () => {
|
||||
const id = await seedQueuedTask(["in-progress", "in-review"]);
|
||||
|
||||
const lease = await h.store().acquireMergeQueueLease("worker-1", { leaseDurationMs: 60_000 });
|
||||
|
||||
expect(lease?.taskId).toBe(id);
|
||||
});
|
||||
|
||||
/*
|
||||
The defect, queue-head mode. Before the fix the head SELECT joined on
|
||||
`tasks.column = 'in-review'`, matched no row on this board, and returned null — the merger saw an
|
||||
empty queue while the card sat in it.
|
||||
*/
|
||||
it("renamed vocabulary: a queued card can be leased by the QUEUE HEAD", async () => {
|
||||
const wf = await seedRenamedWorkflow();
|
||||
const id = await seedQueuedTask(["drafting", "building", "checking"], wf);
|
||||
|
||||
const lease = await h.store().acquireMergeQueueLease("worker-1", { leaseDurationMs: 60_000 });
|
||||
|
||||
expect(lease?.taskId).toBe(id);
|
||||
});
|
||||
|
||||
/*
|
||||
The defect, targeted mode — the path the merger itself uses. Separate predicate, separate failure:
|
||||
the candidate SELECT found nothing, so this recorded `mergeQueue:lease-target-unavailable` and
|
||||
returned null without ever attempting the update.
|
||||
*/
|
||||
it("renamed vocabulary: a queued card can be leased by TARGETED acquire", async () => {
|
||||
const wf = await seedRenamedWorkflow();
|
||||
const id = await seedQueuedTask(["drafting", "building", "checking"], wf);
|
||||
|
||||
const lease = await h
|
||||
.store()
|
||||
.acquireMergeQueueLease("worker-1", { leaseDurationMs: 60_000, targetTaskId: id });
|
||||
|
||||
expect(lease?.taskId).toBe(id);
|
||||
});
|
||||
|
||||
/*
|
||||
The paired negative, and the reason this is a resolved-lane fix rather than a deleted predicate. A
|
||||
card that has LEFT review must not be leaseable — otherwise the merger would merge work that was
|
||||
pulled back for rework. Widening to "any column is a review column" would pass every case above and
|
||||
break exactly this one.
|
||||
*/
|
||||
it("renamed vocabulary: a card moved BACK out of the review lane is not leaseable", async () => {
|
||||
const wf = await seedRenamedWorkflow();
|
||||
const id = await seedQueuedTask(["drafting", "building", "checking"], wf);
|
||||
await h.store().moveTask(id, "building" as never);
|
||||
h.store().taskCache.delete(id);
|
||||
|
||||
const lease = await h
|
||||
.store()
|
||||
.acquireMergeQueueLease("worker-1", { leaseDurationMs: 60_000, targetTaskId: id });
|
||||
|
||||
expect(lease).toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,141 @@
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-02:20 (merge re-enqueue was BROKEN on a renamed board):
|
||||
|
||||
`enqueueMergeQueueInTransaction` gates on the task's column being a review column, and takes the
|
||||
board's review columns as an optional trailing argument. The two `moves.ts` callers — the automatic
|
||||
handoff-to-review path — resolved and passed them. The public `enqueueMergeQueue` wrapper reached
|
||||
through `store.enqueueMergeQueue` did not, so it fell back to `new Set(["in-review"])`.
|
||||
|
||||
The consequence is not the usual quiet legacy-id degradation. The guard's reject branch records
|
||||
`mergeQueue:enqueue-rejected` and THROWS `MergeQueueInvalidColumnError`. So on any board whose review
|
||||
lane is renamed, every re-enqueue through the store method failed outright — and its production
|
||||
callers are `merger.ts` and `self-healing.ts`, i.e. the merge and recovery paths.
|
||||
|
||||
WHY IT SURVIVED. Partial supply. Two of three call sites passed the argument, so a check asking
|
||||
"does SOME caller supply this?" reported the seam as satisfied, and the census counted the
|
||||
conversion. Nothing looked at whether EVERY caller supplied it.
|
||||
|
||||
THE CASES ARE DIFFERENTIAL: the same enqueue against two vocabularies whose roles are identical and
|
||||
only the ids differ. `checking` collides with no legacy literal, so a surviving `"in-review"` cannot
|
||||
pass by luck.
|
||||
*/
|
||||
|
||||
import { it, expect, beforeAll, beforeEach, afterEach, afterAll } from "vitest";
|
||||
import {
|
||||
pgDescribe,
|
||||
createSharedPgTaskStoreTestHarness,
|
||||
type SharedPgTaskStoreHarness,
|
||||
} from "../../__test-utils__/pg-test-harness.js";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR } from "../../index.js";
|
||||
|
||||
pgDescribe("merge-queue enqueue under a renamed review column", () => {
|
||||
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
|
||||
prefix: "fusion_mq_renamed_review",
|
||||
});
|
||||
|
||||
beforeAll(h.beforeAll);
|
||||
beforeEach(h.beforeEach);
|
||||
afterEach(h.afterEach);
|
||||
afterAll(h.afterAll);
|
||||
|
||||
/**
|
||||
* The BUILTIN coding workflow with its column ids renamed — nothing else changed.
|
||||
*
|
||||
* Deriving from the builtin rather than hand-rolling a graph keeps the ONLY difference between the
|
||||
* two runs the vocabulary, which is the whole differential claim. A hand-built fixture ends up
|
||||
* testing the fixture's transition table as much as the code.
|
||||
*/
|
||||
async function seedRenamedWorkflow(): Promise<string> {
|
||||
const RENAME: Record<string, string> = {
|
||||
todo: "drafting",
|
||||
"in-progress": "building",
|
||||
"in-review": "checking",
|
||||
done: "shipped",
|
||||
};
|
||||
const rename = (id: string | undefined) => (id && RENAME[id]) ?? id;
|
||||
const ir = JSON.parse(JSON.stringify(BUILTIN_CODING_WORKFLOW_IR)) as {
|
||||
id: string;
|
||||
nodes?: { column?: string }[];
|
||||
columns?: { id: string }[];
|
||||
};
|
||||
ir.id = "custom:renamed-review-merge-queue";
|
||||
for (const node of ir.nodes ?? []) node.column = rename(node.column);
|
||||
for (const column of ir.columns ?? []) column.id = rename(column.id) as string;
|
||||
|
||||
/* Prove the rename landed: without it a surviving "in-review" literal would pass by accident. */
|
||||
const ids = (ir.columns ?? []).map((column) => column.id);
|
||||
expect(ids).toContain("checking");
|
||||
expect(ids).not.toContain("in-review");
|
||||
|
||||
const created = await h.store().createWorkflowDefinition({
|
||||
name: "Renamed Review (merge queue)",
|
||||
kind: "workflow",
|
||||
ir,
|
||||
} as never);
|
||||
return (created as { id: string }).id;
|
||||
}
|
||||
|
||||
/**
|
||||
* A task walked into the board's review lane. `path` walks the graph rather than jumping, because
|
||||
* moves are transition-validated and a direct hop is rejected under BOTH vocabularies.
|
||||
*/
|
||||
async function seedTaskInReviewLane(path: readonly string[], workflowId?: string): Promise<string> {
|
||||
const store = h.store();
|
||||
const task = await store.createTask({
|
||||
title: "awaiting merge",
|
||||
description: "test",
|
||||
column: "todo",
|
||||
});
|
||||
if (workflowId) await store.writeTaskWorkflowSelection(task.id, workflowId, []);
|
||||
for (const step of path) await store.moveTask(task.id, step as never);
|
||||
store.taskCache.delete(task.id);
|
||||
|
||||
/* Prove the fixture before asserting on it: a task that never reached the review lane would
|
||||
fail to enqueue for a reason that has nothing to do with column vocabulary. */
|
||||
const seeded = await store.getTask(task.id);
|
||||
expect(seeded.column).toBe(path[path.length - 1]);
|
||||
return task.id;
|
||||
}
|
||||
|
||||
/* Control: the default vocabulary enqueues. Passes before and after the fix, so a generally
|
||||
broken enqueue path cannot hide behind the renamed case below. */
|
||||
it("default vocabulary: a task in the review lane enqueues for merge", async () => {
|
||||
const id = await seedTaskInReviewLane(["in-progress", "in-review"]);
|
||||
|
||||
const entry = await h.store().enqueueMergeQueue(id);
|
||||
|
||||
expect(entry.taskId).toBe(id);
|
||||
});
|
||||
|
||||
/*
|
||||
The defect. Before the fix this threw `MergeQueueInvalidColumnError`, because the wrapper omitted
|
||||
the board's review columns and the guard compared `checking` against the legacy `in-review`.
|
||||
*/
|
||||
it("renamed vocabulary: a task in the RENAMED review lane enqueues for merge", async () => {
|
||||
const wf = await seedRenamedWorkflow();
|
||||
const id = await seedTaskInReviewLane(["drafting", "building", "checking"], wf);
|
||||
|
||||
const entry = await h.store().enqueueMergeQueue(id);
|
||||
|
||||
expect(entry.taskId).toBe(id);
|
||||
});
|
||||
|
||||
/*
|
||||
The paired negative, and the reason this is a supply fix rather than a deleted guard: the gate must
|
||||
still REJECT a task that is not in a review lane at all. Wiring the real columns must not degrade
|
||||
into "every column is a review column", which would let work merge straight out of the WIP lane.
|
||||
*/
|
||||
it("renamed vocabulary: a task in the WIP lane is still REJECTED", async () => {
|
||||
const wf = await seedRenamedWorkflow();
|
||||
const id = await seedTaskInReviewLane(["drafting", "building"], wf);
|
||||
|
||||
await expect(h.store().enqueueMergeQueue(id)).rejects.toThrow();
|
||||
});
|
||||
|
||||
/* Same negative under the default vocabulary, so the rejection is not an artifact of the rename. */
|
||||
it("default vocabulary: a task in the WIP lane is still REJECTED", async () => {
|
||||
const id = await seedTaskInReviewLane(["in-progress"]);
|
||||
|
||||
await expect(h.store().enqueueMergeQueue(id)).rejects.toThrow();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,200 @@
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-15:30 (#2819 review — greptile, two findings):
|
||||
|
||||
THE TWO WAYS THIS CONVERSION COULD RE-CREATE THE BUG IT FIXES.
|
||||
|
||||
Both are the same mistake in different clothes — treating "I have no answer" as "the answer is no
|
||||
review lane" — and both land on boards that are not even custom.
|
||||
|
||||
1. TRAITLESS (v1-UPGRADED) BOARDS. `synthesizeDefaultColumns` (workflow-ir.ts:158) upgrades a v1
|
||||
graph by emitting the default column ids with `traits: []`. `resolveReviewColumns` therefore
|
||||
returns EMPTY while the board's `in-review` column plainly exists and holds the card. Forwarding
|
||||
that empty set to the enqueue guard makes it match nothing and throw
|
||||
`MergeQueueInvalidColumnError` — moving the failure off custom boards and onto EVERY pre-v2
|
||||
project. Three states, not two: unreadable and traitless both take the legacy id; only a board
|
||||
that expresses traits and still declares no review lane is answering.
|
||||
|
||||
2. FAILED RESOLUTION DURING THE STALE SWEEP. The sweep's SQL predicate is a candidate superset and
|
||||
each row is verified per task before deletion. The first pass deleted a row whose resolution
|
||||
FAILED, which is the original bug behind a narrower trigger: a transient workflow-read failure
|
||||
silently drops a valid card out of the merge queue, with an audit event claiming
|
||||
`reason: "not-in-review"` for a task sitting in review. Sparing is recoverable (the lease
|
||||
predicates refuse it anyway, and the next sweep removes it once resolution succeeds); deleting
|
||||
is not.
|
||||
|
||||
WHY A LIVE STORE. Both defects live in the seam between a resolver and SQL evaluated by PostgreSQL.
|
||||
A mock supplying a lifecycle struct would assert my own assumption about what the resolver returns,
|
||||
which is the substitution that has produced vacuous tests throughout this program.
|
||||
|
||||
LANE. `.pg.test.ts`, skipped by `pgDescribe` when PostgreSQL is unreachable; throwaway per-file
|
||||
database; never port 4040.
|
||||
*/
|
||||
|
||||
import { it, expect, beforeAll, beforeEach, afterEach, afterAll } from "vitest";
|
||||
import {
|
||||
pgDescribe,
|
||||
createSharedPgTaskStoreTestHarness,
|
||||
type SharedPgTaskStoreHarness,
|
||||
} from "../../__test-utils__/pg-test-harness.js";
|
||||
import { BUILTIN_CODING_WORKFLOW_IR } from "../../index.js";
|
||||
|
||||
pgDescribe("merge queue on traitless and unresolvable workflows", () => {
|
||||
const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({
|
||||
prefix: "fusion_mq_traitless",
|
||||
});
|
||||
|
||||
beforeAll(h.beforeAll);
|
||||
beforeEach(h.beforeEach);
|
||||
afterEach(h.afterEach);
|
||||
afterAll(h.afterAll);
|
||||
|
||||
/**
|
||||
* A v1 graph. The store upgrades it on read, and the upgrade emits the DEFAULT column ids with
|
||||
* EMPTY trait arrays — the exact shape that makes `resolveReviewColumns` return nothing while
|
||||
* `in-review` exists. Written as v1 on purpose: hand-authoring a v2 board with `traits: []` would
|
||||
* be my reconstruction of the upgrade rather than the upgrade itself.
|
||||
*/
|
||||
async function seedV1Workflow(): Promise<string> {
|
||||
const created = await h.store().createWorkflowDefinition({
|
||||
name: "Legacy v1 (traitless after upgrade)",
|
||||
kind: "workflow",
|
||||
ir: {
|
||||
version: "v1",
|
||||
name: "legacy-v1",
|
||||
nodes: [
|
||||
{ id: "start", kind: "start" },
|
||||
{ id: "impl", kind: "agent", seam: "execute" },
|
||||
{ id: "review", kind: "agent", seam: "review" },
|
||||
{ id: "end", kind: "end" },
|
||||
],
|
||||
edges: [
|
||||
{ from: "start", to: "impl" },
|
||||
{ from: "impl", to: "review" },
|
||||
{ from: "review", to: "end" },
|
||||
],
|
||||
},
|
||||
} as never);
|
||||
return (created as { id: string }).id;
|
||||
}
|
||||
|
||||
async function seedTaskInReview(workflowId: string): Promise<string> {
|
||||
const store = h.store();
|
||||
const task = await store.createTask({ title: "v1 card", description: "test", column: "todo" });
|
||||
await store.writeTaskWorkflowSelection(task.id, workflowId, []);
|
||||
for (const step of ["in-progress", "in-review"]) await store.moveTask(task.id, step as never);
|
||||
store.taskCache.delete(task.id);
|
||||
|
||||
/* Prove the fixture before asserting on it. */
|
||||
expect((await store.getTask(task.id)).column).toBe("in-review");
|
||||
return task.id;
|
||||
}
|
||||
|
||||
/*
|
||||
Finding 1. Before the three-state guard this threw `MergeQueueInvalidColumnError`: the upgraded
|
||||
board resolved to an empty review set, so the guard compared `in-review` against nothing.
|
||||
*/
|
||||
it("a v1-upgraded (traitless) board still enqueues a card resting in `in-review`", async () => {
|
||||
const wf = await seedV1Workflow();
|
||||
const id = await seedTaskInReview(wf);
|
||||
|
||||
const entry = await h.store().enqueueMergeQueue(id);
|
||||
|
||||
expect(entry.taskId).toBe(id);
|
||||
});
|
||||
|
||||
/*
|
||||
Finding 1, continued — the enqueue is worth nothing if the row cannot then be leased. The lease
|
||||
predicate resolves the same set, so an empty answer would stall the queue rather than throw, which
|
||||
is the quieter half of the same defect.
|
||||
*/
|
||||
it("a v1-upgraded (traitless) board can then LEASE the queued card", async () => {
|
||||
const wf = await seedV1Workflow();
|
||||
const id = await seedTaskInReview(wf);
|
||||
await h.store().enqueueMergeQueue(id);
|
||||
|
||||
const lease = await h.store().acquireMergeQueueLease("worker-1", { leaseDurationMs: 60_000 });
|
||||
|
||||
expect(lease?.taskId).toBe(id);
|
||||
});
|
||||
|
||||
/**
|
||||
* A RENAMED board, needed for finding 2 and not interchangeable with the v1 one.
|
||||
*
|
||||
* The sweep's SQL selects a CANDIDATE SUPERSET — `column IS DISTINCT FROM 'in-review'` — and only
|
||||
* candidates reach the per-task verification the finding is about. A v1-upgraded board uses the
|
||||
* DEFAULT ids, so its card sits in `in-review` and is never a candidate: written against it, the
|
||||
* case below passes with the fix reverted. I found that by mutating, not by reading.
|
||||
*/
|
||||
async function seedRenamedWorkflow(): Promise<string> {
|
||||
const RENAME: Record<string, string> = {
|
||||
todo: "drafting",
|
||||
"in-progress": "building",
|
||||
"in-review": "checking",
|
||||
done: "shipped",
|
||||
};
|
||||
const rename = (id: string | undefined) => (id && RENAME[id]) ?? id;
|
||||
const ir = JSON.parse(JSON.stringify(BUILTIN_CODING_WORKFLOW_IR)) as {
|
||||
id: string;
|
||||
nodes?: { column?: string }[];
|
||||
columns?: { id: string }[];
|
||||
};
|
||||
ir.id = "custom:traitless-sweep-renamed";
|
||||
for (const node of ir.nodes ?? []) node.column = rename(node.column);
|
||||
for (const column of ir.columns ?? []) column.id = rename(column.id) as string;
|
||||
expect((ir.columns ?? []).map((c) => c.id)).toContain("checking");
|
||||
|
||||
const created = await h.store().createWorkflowDefinition({
|
||||
name: "Renamed (sweep verification)",
|
||||
kind: "workflow",
|
||||
ir,
|
||||
} as never);
|
||||
return (created as { id: string }).id;
|
||||
}
|
||||
|
||||
async function seedRenamedTaskInReview(workflowId: string): Promise<string> {
|
||||
const store = h.store();
|
||||
const task = await store.createTask({ title: "renamed card", description: "test", column: "todo" });
|
||||
await store.writeTaskWorkflowSelection(task.id, workflowId, []);
|
||||
for (const step of ["drafting", "building", "checking"]) await store.moveTask(task.id, step as never);
|
||||
store.taskCache.delete(task.id);
|
||||
expect((await store.getTask(task.id)).column).toBe("checking");
|
||||
return task.id;
|
||||
}
|
||||
|
||||
/*
|
||||
Finding 2. A resolver that THROWS stands in for a transient workflow-read failure. The row must
|
||||
survive the sweep: deletion is unrecoverable, and the acquire that follows refuses the lease anyway
|
||||
because it resolves the same lanes. Asserting the row is still queued afterwards is the point —
|
||||
the previous behaviour deleted it and reported an empty queue as normal.
|
||||
*/
|
||||
it("a row whose workflow cannot be resolved is SPARED by the stale sweep, not deleted", async () => {
|
||||
const wf = await seedRenamedWorkflow();
|
||||
const id = await seedRenamedTaskInReview(wf);
|
||||
await h.store().enqueueMergeQueue(id);
|
||||
|
||||
await h.store().acquireMergeQueueLease("worker-1", {
|
||||
leaseDurationMs: 60_000,
|
||||
resolveReviewColumnsFor: async () => {
|
||||
throw new Error("transient workflow read failure");
|
||||
},
|
||||
});
|
||||
|
||||
expect(await h.store().getMergeQueuedTaskIdsAsync()).toContain(id);
|
||||
});
|
||||
|
||||
/*
|
||||
The paired negative: sparing on failure must not become "never clean anything up". A row whose task
|
||||
has genuinely left review, with resolution WORKING, is still deleted.
|
||||
*/
|
||||
it("a row whose task genuinely left the review lane is still swept", async () => {
|
||||
const wf = await seedRenamedWorkflow();
|
||||
const id = await seedRenamedTaskInReview(wf);
|
||||
await h.store().enqueueMergeQueue(id);
|
||||
await h.store().moveTask(id, "building" as never);
|
||||
h.store().taskCache.delete(id);
|
||||
|
||||
await h.store().acquireMergeQueueLease("worker-1", { leaseDurationMs: 60_000 });
|
||||
|
||||
expect(await h.store().getMergeQueuedTaskIdsAsync()).not.toContain(id);
|
||||
});
|
||||
});
|
||||
@@ -126,7 +126,7 @@ import { createTaskBackendImpl, _createTaskInternalBackendImpl, createTaskImpl,
|
||||
import { getTaskImpl, listTasksImpl, searchTasksImpl, listTasksModifiedSinceImpl, getTaskVerificationRequestAsyncImpl } from "./task-store/reads.js";
|
||||
import { updateTaskUnlockedImpl } from "./task-store/task-update.js";
|
||||
import { __setTaskActivityLogLimitsForTesting } from "./task-store/comments.js";
|
||||
import { resolveReviewColumns, resolveTaskLifecycleColumns, type LifecycleColumns } from "./workflow-lifecycle-traits.js";
|
||||
import { declaresAnyLifecycleTrait, resolveReviewColumns, resolveTaskLifecycleColumns, type LifecycleColumns } from "./workflow-lifecycle-traits.js";
|
||||
import { resolveProjectColumnsForRoles } from "./project-lane-vocabulary.js";
|
||||
import { resolveWorkflowIrForTask } from "./workflow-ir-resolver.js";
|
||||
// FNXC:RuntimeBackendAsync 2026-06-24-10:15:
|
||||
@@ -1449,8 +1449,17 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
The message names the lanes the check actually used, keeping #2709's fix: telling an operator to
|
||||
move to a column their board does not have is worse than refusing.
|
||||
*/
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-15:20 (#2819 review — the same empty-set hole, found by
|
||||
sweeping every `resolveReviewColumns` call site rather than only the one the reviewer named):
|
||||
A v1-upgraded board resolves to an EMPTY review set while its `in-review` column plainly exists,
|
||||
so this guard would refuse the operator's bypass on every pre-v2 project with the unhelpful
|
||||
message "must be in a review lane".
|
||||
*/
|
||||
const reviewIr = await resolveWorkflowIrForTask(this, task.id).catch(() => undefined);
|
||||
const reviewColumns = reviewIr === undefined ? ["in-review"] : resolveReviewColumns(reviewIr);
|
||||
const reviewColumns = reviewIr === undefined || !declaresAnyLifecycleTrait(reviewIr)
|
||||
? ["in-review"]
|
||||
: resolveReviewColumns(reviewIr);
|
||||
if (!reviewColumns.includes(task.column)) {
|
||||
const named = reviewColumns.length > 0 ? reviewColumns.map((c: string) => `'${c}'`).join(" or ") : "a review lane";
|
||||
throw new Error(`Cannot bypass review lane for ${id}: task is in '${task.column}', must be in ${named}`);
|
||||
@@ -1758,7 +1767,26 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
* FNXC:RuntimeLifecycleAsync 2026-06-24-11:20:
|
||||
*/
|
||||
async acquireMergeQueueLease(workerId: string, opts: MergeQueueAcquireOptions): Promise<MergeQueueEntry | null> {
|
||||
return acquireMergeQueueLeaseImpl(this, workerId, opts);
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-01:25 (#2819 review — greptile):
|
||||
Supplies the per-task review-lane resolver for the stale-row sweep. This is the production path,
|
||||
so wiring it here is what makes the option live rather than one only tests fill.
|
||||
*/
|
||||
const withResolver: MergeQueueAcquireOptions = {
|
||||
...opts,
|
||||
resolveReviewColumnsFor: opts.resolveReviewColumnsFor ?? (async (taskId: string) => {
|
||||
/*
|
||||
Three states, not two. `undefined` is "could not read"; an IR whose columns carry NO
|
||||
lifecycle trait at all is a v1 graph upgraded by `synthesizeDefaultColumns`, whose
|
||||
`in-review` column plainly exists and holds cards. Both take the legacy answer. Only a
|
||||
board that expresses traits and still has no review lane is answering the question.
|
||||
*/
|
||||
const ir = await resolveWorkflowIrForTask(this, taskId).catch(() => undefined);
|
||||
if (!ir || !declaresAnyLifecycleTrait(ir)) return new Set(["in-review"]);
|
||||
return new Set(resolveReviewColumns(ir));
|
||||
}),
|
||||
};
|
||||
return acquireMergeQueueLeaseImpl(this, workerId, withResolver);
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -97,22 +97,49 @@ function leaseAvailable(now: string) {
|
||||
}
|
||||
|
||||
/**
|
||||
* Predicate: the queue row's task is still in the `in-review` column.
|
||||
* Predicate: the queue row's task is still in a REVIEW lane.
|
||||
*
|
||||
* FNXC:MultiProjectIsolation 2026-07-10: when `projectId` is bound, the EXISTS
|
||||
* additionally requires the task to belong to this project so a project's
|
||||
* merger can only lease its OWN queue rows (merge_queue has no project_id, so
|
||||
* it is scoped transitively through its task on the shared embedded-PG cluster).
|
||||
*
|
||||
* FNXC:WorkflowResolvedColumns 2026-07-30-13:55 (#2819 review — greptile,
|
||||
* "renamed entries remain unleaseable"):
|
||||
*
|
||||
* THE STALE SWEEP SPARED RENAMED ROWS AND THE LEASE PREDICATE THEN REFUSED THEM.
|
||||
*
|
||||
* The first pass fixed only `cleanupStaleMergeQueueRowsInTransaction`, so on a
|
||||
* board whose review lane is called `signoff` the queue row survived — and then
|
||||
* both eligibility predicates (targeted acquire and queue-head acquire) still
|
||||
* demanded the literal `in-review`, matched nothing, and the row sat forever.
|
||||
* Sparing a row you will never lease is worse than deleting it: deletion at
|
||||
* least surfaces as an audit event, while this is a silent permanent stall.
|
||||
*
|
||||
* THE LANES ARE INJECTED INTO THE SQL rather than verified after the fact. The
|
||||
* WHERE clause here is also the concurrency control — the acquire re-checks it
|
||||
* inside `UPDATE ... WHERE` so a racing worker updates zero rows. Verifying the
|
||||
* column in JS between SELECT and UPDATE would move that check outside the
|
||||
* atomic step and let two workers lease the same task. `reviewColumns` is
|
||||
* therefore a resolved set the caller supplies, and it goes into the predicate.
|
||||
*/
|
||||
function taskStillInReview(projectId?: string) {
|
||||
function taskStillInReview(projectId?: string, reviewColumns?: ReadonlySet<string>) {
|
||||
const projectClause = projectId
|
||||
? sql`AND ${schema.project.tasks.projectId} = ${projectId}`
|
||||
: sql``;
|
||||
const lanes = [...(reviewColumns ?? new Set(["in-review"]))];
|
||||
/*
|
||||
An EMPTY resolved set means the board declares no review lane at all. `inArray` with no
|
||||
values is not portable across drizzle versions (it has both thrown and folded to FALSE),
|
||||
so the FALSE is written explicitly. Nothing can be enqueued on such a board either — the
|
||||
enqueue guard resolves the same set — so an unleaseable queue there is not a new stall.
|
||||
*/
|
||||
const laneClause = lanes.length > 0 ? inArray(schema.project.tasks.column, lanes) : sql`FALSE`;
|
||||
return sql<boolean>`
|
||||
EXISTS (
|
||||
SELECT 1 FROM ${schema.project.tasks}
|
||||
WHERE ${schema.project.tasks.id} = ${schema.project.mergeQueue.taskId}
|
||||
AND ${schema.project.tasks.column} = 'in-review'
|
||||
AND ${laneClause}
|
||||
${projectClause}
|
||||
)
|
||||
`;
|
||||
@@ -241,9 +268,19 @@ export async function enqueueMergeQueue(
|
||||
taskId: string,
|
||||
opts: MergeQueueEnqueueOptions = {},
|
||||
audit?: { agentId?: string; runId?: string },
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-02:10:
|
||||
The board's review columns, forwarded to the in-transaction helper. Omitted, that helper falls back
|
||||
to `new Set(["in-review"])` and REJECTS a task resting in a renamed review column — it is not a
|
||||
quiet legacy-id degradation like most of these seams, it records `mergeQueue:enqueue-rejected` and
|
||||
throws `MergeQueueInvalidColumnError`. Both `moves.ts` callers already resolve and pass this; this
|
||||
wrapper did not, so the automatic handoff-to-review path worked on a renamed board while the manual
|
||||
and recovery re-enqueue paths through `store.enqueueMergeQueue` (merger.ts, self-healing.ts) threw.
|
||||
*/
|
||||
reviewColumns?: ReadonlySet<string>,
|
||||
): Promise<MergeQueueEntry> {
|
||||
return layer.transactionImmediate((tx) =>
|
||||
enqueueMergeQueueInTransaction(tx, taskId, opts, audit),
|
||||
enqueueMergeQueueInTransaction(tx, taskId, opts, audit, reviewColumns),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -260,6 +297,13 @@ export async function enqueueMergeQueue(
|
||||
export async function cleanupStaleMergeQueueRowsInTransaction(
|
||||
tx: DbTransaction,
|
||||
now: string,
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-01:20 (#2819 review — greptile):
|
||||
Per-task review lanes, supplied by the caller. This runs inside an open transaction with only a
|
||||
`tx` handle, so it cannot resolve a workflow itself — and the lanes are genuinely per task, because
|
||||
queued tasks can run different workflows.
|
||||
*/
|
||||
resolveReviewColumnsFor?: (taskId: string) => Promise<ReadonlySet<string>>,
|
||||
): Promise<void> {
|
||||
const staleRows = await tx
|
||||
.select({
|
||||
@@ -277,7 +321,42 @@ export async function cleanupStaleMergeQueueRowsInTransaction(
|
||||
),
|
||||
);
|
||||
|
||||
if (staleRows.length === 0) return;
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-01:20 (#2819 review — greptile):
|
||||
THE SQL PREDICATE IS A SUPERSET NOW, NOT THE VERDICT.
|
||||
|
||||
`column IS DISTINCT FROM 'in-review'` is evaluated by PostgreSQL, which cannot know a task's
|
||||
workflow — so on a board whose review lane is named anything else, EVERY queued row matched and was
|
||||
deleted at the start of lease acquisition. The merge queue filled (enqueue resolves lanes) and was
|
||||
then emptied a moment later by this sweep, so nothing was ever merged and no error was raised: the
|
||||
audit rows even claimed `reason: "not-in-review"` for tasks sitting in their board's review lane.
|
||||
|
||||
Deleting is the destructive direction, so the SQL now only proposes CANDIDATES and each one is
|
||||
verified against its own workflow before removal. Rows whose task is gone are stale regardless and
|
||||
skip the check.
|
||||
*/
|
||||
const verifiedStale = resolveReviewColumnsFor === undefined
|
||||
? staleRows
|
||||
: (await Promise.all(staleRows.map(async (row) => {
|
||||
if (row.column === null) return row;
|
||||
const reviewLanes = await resolveReviewColumnsFor(row.taskId).catch(() => undefined);
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-15:10 (#2819 review — greptile,
|
||||
"resolution failures delete valid rows"):
|
||||
A FAILED RESOLUTION SPARES THE ROW. The first pass returned the row here, i.e. deleted it —
|
||||
which reintroduced the exact bug this sweep fix exists to remove, just behind a narrower
|
||||
trigger: any transient workflow-read failure silently dropped a valid card out of the merge
|
||||
queue, and the audit event claimed `reason: "not-in-review"` for a task sitting in review.
|
||||
|
||||
Sparing is safe in a way deleting is not. The lease predicates resolve the same lanes, so a
|
||||
row spared here but genuinely stale is still refused a lease, and the next acquire re-runs
|
||||
this sweep and removes it once resolution succeeds. Deleting is unrecoverable.
|
||||
*/
|
||||
if (reviewLanes === undefined) return undefined;
|
||||
return reviewLanes.has(row.column) ? undefined : row;
|
||||
}))).filter((row): row is typeof staleRows[number] => row !== undefined);
|
||||
|
||||
if (verifiedStale.length === 0) return;
|
||||
|
||||
// FNXC:TaskStoreMergeCoordination 2026-06-26-10:10:
|
||||
// Batch the cleanup to avoid an N+1: previously each stale row cost 2
|
||||
@@ -286,12 +365,12 @@ export async function cleanupStaleMergeQueueRowsInTransaction(
|
||||
// acquired. Now the deletes are a single bulk DELETE ... WHERE IN (...) and
|
||||
// the audit events are a single bulk INSERT ... VALUES (...). Each metadata
|
||||
// payload is still per-row (the column/lease context differs per task).
|
||||
const staleTaskIds = staleRows.map((row) => row.taskId);
|
||||
const staleTaskIds = verifiedStale.map((row) => row.taskId);
|
||||
await tx
|
||||
.delete(schema.project.mergeQueue)
|
||||
.where(inArray(schema.project.mergeQueue.taskId, staleTaskIds));
|
||||
|
||||
const auditValues = staleRows.map((row) => ({
|
||||
const auditValues = verifiedStale.map((row) => ({
|
||||
id: randomUUID(),
|
||||
timestamp: now,
|
||||
taskId: row.taskId,
|
||||
@@ -353,20 +432,37 @@ export async function acquireMergeQueueLease(
|
||||
// must be scoped to this project so a project's merger can never lease (and
|
||||
// then merge in the wrong repo) another project's in-review task.
|
||||
const projectId = layer.projectId;
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-14:05 (#2819 review):
|
||||
One resolution per task id per acquire, memoised, because the same id is asked
|
||||
about twice on the targeted path (candidate SELECT, then the atomic UPDATE) and
|
||||
the two MUST agree — resolving twice and getting different answers would let the
|
||||
UPDATE's predicate diverge from the one that admitted the candidate.
|
||||
*/
|
||||
const laneCache = new Map<string, ReadonlySet<string>>();
|
||||
const reviewLanesFor = async (taskId: string): Promise<ReadonlySet<string> | undefined> => {
|
||||
if (!opts.resolveReviewColumnsFor) return undefined;
|
||||
const cached = laneCache.get(taskId);
|
||||
if (cached) return cached;
|
||||
const resolved = await opts.resolveReviewColumnsFor(taskId).catch(() => undefined);
|
||||
if (resolved) laneCache.set(taskId, resolved);
|
||||
return resolved;
|
||||
};
|
||||
return layer.transactionImmediate(async (tx) => {
|
||||
const now = opts.now ?? new Date().toISOString();
|
||||
const leaseExpiresAt = new Date(Date.parse(now) + opts.leaseDurationMs).toISOString();
|
||||
await cleanupStaleMergeQueueRowsInTransaction(tx, now);
|
||||
await cleanupStaleMergeQueueRowsInTransaction(tx, now, opts.resolveReviewColumnsFor);
|
||||
|
||||
if (opts.targetTaskId) {
|
||||
// ── Targeted acquire: lease this specific task or fail ──────────────
|
||||
const targetLanes = await reviewLanesFor(opts.targetTaskId);
|
||||
const candidateRows = await tx
|
||||
.select({ taskId: schema.project.mergeQueue.taskId })
|
||||
.from(schema.project.mergeQueue)
|
||||
.where(
|
||||
and(
|
||||
eq(schema.project.mergeQueue.taskId, opts.targetTaskId),
|
||||
taskStillInReview(projectId),
|
||||
taskStillInReview(projectId, targetLanes),
|
||||
leaseAvailable(now),
|
||||
),
|
||||
)
|
||||
@@ -419,7 +515,7 @@ export async function acquireMergeQueueLease(
|
||||
.where(
|
||||
and(
|
||||
eq(schema.project.mergeQueue.taskId, opts.targetTaskId),
|
||||
taskStillInReview(projectId),
|
||||
taskStillInReview(projectId, targetLanes),
|
||||
leaseAvailable(now),
|
||||
),
|
||||
)
|
||||
@@ -448,10 +544,27 @@ export async function acquireMergeQueueLease(
|
||||
return entry;
|
||||
}
|
||||
|
||||
// ── Queue-head acquire: lease the highest-priority, earliest available row ──
|
||||
// Select the candidate first (priority-first, FIFO within priority), then
|
||||
// UPDATE it while re-checking availability to avoid a lost-update race.
|
||||
const headRows = await tx
|
||||
/*
|
||||
── Queue-head acquire: lease the highest-priority, earliest available row ──
|
||||
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-14:20 (#2819 review):
|
||||
THE HEAD SELECT CANNOT NAME THE LANES, BECAUSE IT DOES NOT YET KNOW THE TASK.
|
||||
Each queued row can run a DIFFERENT workflow, so "is this row in review?" has a
|
||||
different answer per row and the resolver is keyed by task id. The select is
|
||||
therefore a CANDIDATE SUPERSET — ordered exactly as before — and each candidate
|
||||
is resolved in turn until one qualifies. The chosen row's own resolved lanes then
|
||||
go into the UPDATE's WHERE, so the atomic re-check is preserved: the row is only
|
||||
leased if it is STILL both available and in one of ITS review lanes.
|
||||
|
||||
Without a resolver (callers that cannot resolve workflows) the predicate stays
|
||||
the literal and the loop stops at the first candidate — the previous behaviour.
|
||||
|
||||
The candidate scan is bounded. The stale sweep above has already deleted rows
|
||||
whose task left review, so under normal operation the first candidate qualifies;
|
||||
the bound only caps the pathological case where the sweep could not resolve.
|
||||
*/
|
||||
const HEAD_CANDIDATE_SCAN_LIMIT = 25;
|
||||
const candidateHeadRows = await tx
|
||||
.select({ taskId: schema.project.mergeQueue.taskId })
|
||||
.from(schema.project.mergeQueue)
|
||||
.innerJoin(
|
||||
@@ -460,15 +573,35 @@ export async function acquireMergeQueueLease(
|
||||
)
|
||||
.where(
|
||||
and(
|
||||
eq(schema.project.tasks.column, "in-review"),
|
||||
// FNXC:MultiProjectIsolation 2026-07-10: only this project's tasks.
|
||||
taskProjectScope(layer),
|
||||
leaseAvailable(now),
|
||||
),
|
||||
)
|
||||
.orderBy(MERGE_QUEUE_PRIORITY_RANK, schema.project.mergeQueue.enqueuedAt)
|
||||
.limit(1);
|
||||
const head = headRows[0];
|
||||
.limit(opts.resolveReviewColumnsFor ? HEAD_CANDIDATE_SCAN_LIMIT : 1);
|
||||
|
||||
let head: { taskId: string } | undefined;
|
||||
let headLanes: ReadonlySet<string> | undefined;
|
||||
for (const candidate of candidateHeadRows) {
|
||||
const lanes = await reviewLanesFor(candidate.taskId);
|
||||
const eligible = await tx
|
||||
.select({ taskId: schema.project.mergeQueue.taskId })
|
||||
.from(schema.project.mergeQueue)
|
||||
.where(
|
||||
and(
|
||||
eq(schema.project.mergeQueue.taskId, candidate.taskId),
|
||||
taskStillInReview(projectId, lanes),
|
||||
leaseAvailable(now),
|
||||
),
|
||||
)
|
||||
.limit(1);
|
||||
if (eligible.length > 0) {
|
||||
head = candidate;
|
||||
headLanes = lanes;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!head) {
|
||||
return null;
|
||||
}
|
||||
@@ -483,6 +616,7 @@ export async function acquireMergeQueueLease(
|
||||
.where(
|
||||
and(
|
||||
eq(schema.project.mergeQueue.taskId, head.taskId),
|
||||
taskStillInReview(projectId, headLanes),
|
||||
leaseAvailable(now),
|
||||
),
|
||||
)
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
|
||||
import { TaskStore } from "../store.js";
|
||||
import { resolveProjectColumnsForRoles } from "../project-lane-vocabulary.js";
|
||||
import {resolveTaskLifecycleColumns} from "../workflow-lifecycle-traits.js";
|
||||
import {declaresAnyLifecycleTrait, resolveReviewColumns, resolveTaskLifecycleColumns} from "../workflow-lifecycle-traits.js";
|
||||
import {resolveWorkflowIrForTask} from "../workflow-ir-resolver.js";
|
||||
import { countAgentLogEntries, readAgentLogEntries } from "../agent-log-file-store.js";
|
||||
import { toJsonNullable } from "../db.js";
|
||||
@@ -85,7 +85,31 @@ export async function enqueueMergeQueueImpl(store: TaskStore, taskId: string, op
|
||||
FNXC:SqliteDualPathCleanup 2026-07-26-14:05:
|
||||
Merge-queue enqueue is PostgreSQL-only via enqueueMergeQueueAsync (column check, idempotent insert, audit). The SQLite enqueueMergeQueueSyncInternal arm is deleted.
|
||||
*/
|
||||
return enqueueMergeQueueAsync(store.asyncLayer!, taskId, opts);
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-02:10:
|
||||
Resolve the task's OWN review columns before enqueueing. Without them the in-transaction guard
|
||||
falls back to `new Set(["in-review"])` and throws `MergeQueueInvalidColumnError` for a task
|
||||
resting in a renamed review lane, which breaks the merger and self-healing re-enqueue paths on
|
||||
every custom board. `undefined` on failure is deliberate and matches `moves.ts`: it makes the
|
||||
guard fall back to the legacy id rather than to an empty set that matches nothing.
|
||||
*/
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-15:15 (#2819 review — greptile, "traitless workflows reject re-enqueue"):
|
||||
THREE STATES, NOT TWO. `undefined` (unreadable) was already handled. The missing one is an IR that
|
||||
resolves but carries NO lifecycle trait on any column: `synthesizeDefaultColumns` upgrades a v1
|
||||
graph by emitting the default columns with `traits: []`, so `resolveReviewColumns` returns EMPTY
|
||||
while the board's `in-review` column plainly exists and holds the card. Forwarding that empty set
|
||||
made the guard match nothing and throw `MergeQueueInvalidColumnError` — the very failure this
|
||||
conversion was fixing, moved from custom boards onto every pre-v2 project.
|
||||
|
||||
Only a board that EXPRESSES traits and still has no review lane is answering the question; the
|
||||
other two states take the legacy id.
|
||||
*/
|
||||
const reviewIr = await resolveWorkflowIrForTask(store, taskId).catch(() => undefined);
|
||||
const reviewColumns = reviewIr && declaresAnyLifecycleTrait(reviewIr)
|
||||
? new Set(resolveReviewColumns(reviewIr))
|
||||
: undefined;
|
||||
return enqueueMergeQueueAsync(store.asyncLayer!, taskId, opts, undefined, reviewColumns);
|
||||
}
|
||||
|
||||
export function cleanupStaleMergeQueueRowsImpl(store: TaskStore, now: string): void {
|
||||
|
||||
@@ -171,6 +171,17 @@ export interface MergeQueueAcquireOptions {
|
||||
/** If provided, the lease attempt targets this specific task first.
|
||||
* The task must be unexpired/available; otherwise falls back to normal queue-head selection. */
|
||||
targetTaskId?: string;
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-01:25 (#2819 review — greptile):
|
||||
Resolves a task's review lanes for the stale-row sweep that runs at the start of lease acquisition.
|
||||
That sweep deletes rows whose task has left review, and its predicate is evaluated by PostgreSQL —
|
||||
which cannot know a workflow. Without this the SQL literal deleted every queued row on a renamed
|
||||
board, so the queue filled and emptied and nothing merged.
|
||||
|
||||
Optional so a caller with only a data layer keeps today's behaviour; `TaskStore.acquireMergeQueueLease`
|
||||
supplies it, which is the path production uses.
|
||||
*/
|
||||
resolveReviewColumnsFor?: (taskId: string) => Promise<ReadonlySet<string>>;
|
||||
}
|
||||
|
||||
export type MergeQueueReleaseOutcome =
|
||||
|
||||
Reference in New Issue
Block a user