fix(core): the merge-queue enqueue guard was not debt — the code it guarded had no callers (#3205)
## The deferral note was right about the mechanism and wrong about the
remedy
`merge-queue-ops-2.ts` sat in the census as deferred debt behind this
note:
> Converting it properly means either making this path async or pushing
the trait read into SQL, both of which are store-architecture changes
rather than call-site conversions.
That is correct as far as it goes — the guard runs inside
`store.db.transactionImmediate`, so the only synchronous resolver
available (`resolveTaskWorkflowIrSync`) returns the DEFAULT workflow
under PostgreSQL and a "conversion" would be inert.
But it assumed the code needed converting. Measured across the tree:
```
=== every call site of .enqueueMergeQueueSyncInternal( ===
packages/core/src/store.ts:1775: public enqueueMergeQueueSyncInternal(...) <- the declaration itself
```
**Zero callers.** Every other occurrence of the name is a comment. The
live path is `enqueueMergeQueueAsync` (`task-artifacts-ops.ts:117`), and
that file already documented the deletion:
> Merge-queue enqueue is PostgreSQL-only via enqueueMergeQueueAsync …
The SQLite `enqueueMergeQueueSyncInternal` arm is deleted.
The arm was deleted; its declaration was not. The guard was unreachable
on the shipped backend.
## Change
- Deleted `enqueueMergeQueueSyncInternalImpl` (-85 lines) and its
`store.enqueueMergeQueueSyncInternal` entry point.
- Dropped the six imports that became unused
(`MergeQueueTaskNotFoundError`, `MergeQueueInvalidColumnError`,
`MergeQueueEntry`, `MergeQueueEnqueueOptions`, `normalizeTaskPriority`,
`MergeQueueRow`).
- Refreshed the three comments naming the removed symbol, so none points
at a deleted identifier. The
`handoffMergeQueueFailureInjectorForTesting` hook those comments sit on
is a **different** member and is untouched — it only mentioned the sync
arm as context.
## Census before / after
| | before | after |
|---|---|---|
| `packages/core/src/task-store/merge-queue-ops-2.ts` | 1 | **0 (entry
removed)** |
Baseline tightened by exactly one entry. **The 0 here is a deletion, not
a conversion** — recorded in the file's own FNXC note so the next worker
does not read it as a converted seam. This is the failure mode the
census warns about ("a count of 0 is the WORST case, not the best"), so
it is stated at the site rather than left to inference.
## Measured
| check | result |
|---|---|
| `census --strict` | exit 0 |
| `@fusion/core tsc --noEmit` | exit 0 |
| `eslint` (4 changed files) | clean |
| core merge-queue tests | **110 passed / 6 files**, incl.
`postgres/merge-queue-renamed-review-column.pg.test.ts` |
| `pnpm test:gate` | exit 0 (**732 tests**) |
No changeset: `@fusion/core` is private and this removes unreachable
code with no user-visible behavior.
## Flagged, not guessed
The other four deferral-note files remain deferred. I only reclassified
this one because its call-site count is a fact I could measure, not a
judgement. Whether `lifecycle-ops.ts:667` is likewise dead (it sits in
the legacy-SQLite polling-replica path) is a separate question I have
not measured, so I have not touched it.
This commit is contained in:
@@ -109,7 +109,7 @@ import { resolveTaskSymbolsForTask, type TaskSymbolResolution } from "./task-sym
|
||||
import { acquireSymbolLocksAsync, inspectSymbolLockConflictsAsync, reconcileStaleSymbolLocksAsync, releaseSymbolLocksAsync, renewSymbolLocksAsync } from "./task-store/symbol-locks.js";
|
||||
import type { AcquireSymbolLocksResult, ReconcileStaleSymbolLocksResult, ReleaseSymbolLocksResult, RenewSymbolLocksResult, SymbolLockConflict, SymbolLockOwner } from "./symbol-lock-types.js";
|
||||
import { queryRunAuditEvents } from "./task-store/async-audit.js";
|
||||
import { isValidMergeRequestTransitionImpl, enqueueMergeQueueSyncInternalImpl, releaseMergeQueueLeaseImpl, collectMergeDetailsImpl, applyPrMergedTransitionImpl } from "./task-store/merge-queue-ops-2.js";
|
||||
import { isValidMergeRequestTransitionImpl, releaseMergeQueueLeaseImpl, collectMergeDetailsImpl, applyPrMergedTransitionImpl } from "./task-store/merge-queue-ops-2.js";
|
||||
import { upsertWorkflowWorkItemImpl, replaceActiveTaskWorkflowContinuationImpl, seedStrandedPlanReviewContinuationImpl, transitionWorkflowWorkItemImpl, acquireWorkflowWorkItemLeaseImpl } from "./task-store/workflow-workitems-ops-2.js";
|
||||
import { getSettingsImpl, getSettingsFastImpl, getSettingsByScopeImpl, getSettingsByScopeFastImpl } from "./task-store/settings-ops-2.js";
|
||||
import { runPluginColumnTransitionHooksImpl, logEntryImpl } from "./task-store/audit-ops.js";
|
||||
@@ -388,8 +388,9 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
|
||||
/*
|
||||
FNXC:HandoffFailureInjection 2026-07-15-12:00:
|
||||
PostgreSQL handoffs call enqueueMergeQueueInTransaction directly, bypassing the
|
||||
legacy enqueueMergeQueueSyncInternal spy. Keep this test-only hook dormant in
|
||||
PostgreSQL handoffs call enqueueMergeQueueInTransaction directly. The legacy sync
|
||||
SQLite enqueue arm this once bypassed was deleted 2026-07-31 (it had no callers);
|
||||
this hook is unrelated to it and stays. Keep this test-only hook dormant in
|
||||
production so VAL-DATA-013 can inject a late transaction failure and prove every
|
||||
handoff sub-write rolls back without adding queries or runtime behavior.
|
||||
*/
|
||||
@@ -1783,12 +1784,6 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
return enqueueMergeQueueImpl(this, taskId, opts);
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:RuntimeLifecycleAsync 2026-06-24-11:15:
|
||||
*/
|
||||
public enqueueMergeQueueSyncInternal(taskId: string, opts: MergeQueueEnqueueOptions): MergeQueueEntry {
|
||||
return enqueueMergeQueueSyncInternalImpl(this, taskId, opts);
|
||||
}
|
||||
public cleanupStaleMergeQueueRows(now: string): void {
|
||||
return cleanupStaleMergeQueueRowsImpl(this, now);
|
||||
}
|
||||
|
||||
@@ -7,14 +7,11 @@
|
||||
* instance as its first parameter and performs byte-identical work.
|
||||
*/
|
||||
import {TaskStore, storeLog} from "../store.js";
|
||||
import {MergeQueueTaskNotFoundError, MergeQueueInvalidColumnError} from "./errors.js";
|
||||
import type {Task, Column, MergeResult, MergeQueueEntry, MergeQueueEnqueueOptions, MergeQueueReleaseOutcome, MergeRequestState} from "../types.js";
|
||||
import type {Task, Column, MergeResult, MergeQueueReleaseOutcome, MergeRequestState} from "../types.js";
|
||||
import "../builtin-traits.js";
|
||||
import {normalizeTaskPriority} from "../task-priority.js";
|
||||
import {resolveTaskLifecycleColumns} from "../workflow-lifecycle-traits.js";
|
||||
import {__setTaskActivityLogLimitsForTesting} from "../task-store/comments.js";
|
||||
import {releaseMergeQueueLease as releaseMergeQueueLeaseAsync} from "../task-store/async-merge-coordination.js";
|
||||
import type {MergeQueueRow} from "../task-store/row-types.js";
|
||||
|
||||
export function isValidMergeRequestTransitionImpl(store: TaskStore, from: MergeRequestState, to: MergeRequestState): boolean {
|
||||
if (from === to) return true;
|
||||
@@ -30,91 +27,15 @@ export function isValidMergeRequestTransitionImpl(store: TaskStore, from: MergeR
|
||||
return allowed[from].has(to);
|
||||
}
|
||||
|
||||
export function enqueueMergeQueueSyncInternalImpl(store: TaskStore, taskId: string, opts: MergeQueueEnqueueOptions): MergeQueueEntry {
|
||||
let invalidColumn: Column | null = null;
|
||||
const entry = store.db.transactionImmediate(() => {
|
||||
const existing = store.db.prepare("SELECT * FROM mergeQueue WHERE taskId = ?").get(taskId) as MergeQueueRow | undefined;
|
||||
const taskRow = store.db.prepare("SELECT priority, column FROM tasks WHERE id = ?").get(taskId) as { priority: string | null; column: Column } | undefined;
|
||||
if (!taskRow) {
|
||||
throw new MergeQueueTaskNotFoundError(taskId);
|
||||
}
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-08-02-10:35 (fleet — FLAGGED, deliberately NOT converted):
|
||||
This guard runs inside `store.db.transactionImmediate`, a SYNCHRONOUS SQLite transaction. Converting
|
||||
it needs a synchronous lane resolution, and the only one available (`resolveTaskWorkflowIrSync`) reads
|
||||
`getTaskWorkflowSelectionImpl`, which returns `undefined` unconditionally in PostgreSQL mode — the
|
||||
shipped backend. So a "conversion" here would drop the census count by one and behave exactly as the
|
||||
literal does (documented on `isBenignInReviewPauseAbort` in executor.ts, PR #2703).
|
||||
|
||||
Converting it properly means either making this path async or pushing the trait read into SQL, both of
|
||||
which are store-architecture changes rather than call-site conversions. Left as a literal WITH this
|
||||
note so the next worker does not "fix" it into a false green.
|
||||
*/
|
||||
if (taskRow.column !== "in-review") {
|
||||
invalidColumn = taskRow.column;
|
||||
return null;
|
||||
}
|
||||
|
||||
const now = opts.now ?? new Date().toISOString();
|
||||
const priority = opts.priority ?? normalizeTaskPriority(taskRow.priority);
|
||||
|
||||
let nextEntry: MergeQueueEntry;
|
||||
let alreadyEnqueued = true;
|
||||
if (existing) {
|
||||
nextEntry = store.rowToMergeQueueEntry(existing);
|
||||
} else {
|
||||
store.db.prepare(`
|
||||
INSERT INTO mergeQueue (taskId, enqueuedAt, priority, attemptCount)
|
||||
VALUES (?, ?, ?, 0)
|
||||
ON CONFLICT(taskId) DO NOTHING
|
||||
`).run(taskId, now, priority);
|
||||
const inserted = store.db.prepare("SELECT * FROM mergeQueue WHERE taskId = ?").get(taskId) as MergeQueueRow | undefined;
|
||||
if (!inserted) {
|
||||
throw new Error(`Failed to read merge queue entry for ${taskId} after enqueue`);
|
||||
}
|
||||
nextEntry = store.rowToMergeQueueEntry(inserted);
|
||||
alreadyEnqueued = false;
|
||||
}
|
||||
|
||||
store.insertRunAuditEventRow({
|
||||
taskId,
|
||||
domain: "database",
|
||||
mutationType: "mergeQueue:enqueue",
|
||||
target: taskId,
|
||||
metadata: {
|
||||
taskId,
|
||||
priority: nextEntry.priority,
|
||||
enqueuedAt: nextEntry.enqueuedAt,
|
||||
alreadyEnqueued,
|
||||
},
|
||||
});
|
||||
|
||||
return nextEntry;
|
||||
});
|
||||
|
||||
if (invalidColumn) {
|
||||
store.db.transactionImmediate(() => {
|
||||
store.insertRunAuditEventRow({
|
||||
taskId,
|
||||
domain: "database",
|
||||
mutationType: "mergeQueue:enqueue-rejected",
|
||||
target: taskId,
|
||||
metadata: {
|
||||
taskId,
|
||||
column: invalidColumn,
|
||||
reason: "not-in-review",
|
||||
},
|
||||
});
|
||||
});
|
||||
throw new MergeQueueInvalidColumnError(taskId, invalidColumn);
|
||||
}
|
||||
|
||||
if (!entry) {
|
||||
throw new Error(`Failed to enqueue merge queue entry for ${taskId}`);
|
||||
}
|
||||
return entry;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-10:00 (u12 — DELETED, not converted):
|
||||
`enqueueMergeQueueSyncInternalImpl` and its `store.enqueueMergeQueueSyncInternal` entry point are gone.
|
||||
Merge-queue enqueue is PostgreSQL-only via `enqueueMergeQueueAsync` (task-artifacts-ops.ts); the sync
|
||||
SQLite arm had ZERO callers — every remaining mention was a comment. Its `column !== "in-review"` guard
|
||||
was carried in the census as deferred debt "needing a store-architecture change to convert". It needed
|
||||
no conversion: the code it guarded was unreachable on the shipped backend. Removing dead code is why the
|
||||
count drops here, so do not read this file's 0 as a converted seam.
|
||||
*/
|
||||
export async function releaseMergeQueueLeaseImpl(store: TaskStore, taskId: string, workerId: string, outcome: MergeQueueReleaseOutcome): Promise<void> {
|
||||
const layer = store.asyncLayer!;
|
||||
return releaseMergeQueueLeaseAsync(layer, taskId, workerId, outcome);
|
||||
|
||||
@@ -564,7 +564,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum
|
||||
});
|
||||
/*
|
||||
FNXC:HandoffFailureInjection 2026-07-15-12:00:
|
||||
Backend handoffs bypass the legacy enqueueMergeQueueSyncInternal spy.
|
||||
The legacy sync SQLite enqueue arm was deleted 2026-07-31 (no callers).
|
||||
This test-only no-op seam runs after every VAL-DATA-013 sub-write
|
||||
(move, queue, workflow work, and handoff audit), so an injected throw
|
||||
proves this transaction rolls all of them back.
|
||||
@@ -1243,7 +1243,7 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum
|
||||
});
|
||||
/*
|
||||
FNXC:HandoffFailureInjection 2026-07-15-12:00:
|
||||
Backend handoffs bypass the legacy enqueueMergeQueueSyncInternal spy.
|
||||
The legacy sync SQLite enqueue arm was deleted 2026-07-31 (no callers).
|
||||
This test-only no-op seam runs after every VAL-DATA-013 sub-write
|
||||
(move, queue, workflow work, and handoff audit), so an injected throw
|
||||
proves this transaction rolls all of them back.
|
||||
|
||||
@@ -88,7 +88,7 @@ export async function recordPluginActivationImpl(store: TaskStore, input: Plugin
|
||||
export async function enqueueMergeQueueImpl(store: TaskStore, taskId: string, opts: MergeQueueEnqueueOptions = {}): Promise<MergeQueueEntry> {
|
||||
/*
|
||||
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.
|
||||
Merge-queue enqueue is PostgreSQL-only via enqueueMergeQueueAsync (column check, idempotent insert, audit). The SQLite sync arm is deleted (source removed 2026-07-31).
|
||||
*/
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-02:10:
|
||||
|
||||
@@ -4,7 +4,6 @@
|
||||
"packages/engine/src/scheduler.ts": 2,
|
||||
"packages/core/src/task-store/audit-ops.ts": 1,
|
||||
"packages/core/src/task-store/lifecycle-ops.ts": 1,
|
||||
"packages/core/src/task-store/merge-queue-ops-2.ts": 1,
|
||||
"packages/core/src/task-store/moves.ts": 1,
|
||||
"packages/core/src/task-store/task-id-integrity.ts": 1,
|
||||
"packages/dashboard/app/components/ResearchTaskActionModal.tsx": 1,
|
||||
|
||||
Reference in New Issue
Block a user