From 230be28576c52da143ed9fc9e90a93e7344b07e6 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 31 Jul 2026 10:30:16 -0700 Subject: [PATCH] =?UTF-8?q?fix(core):=20the=20merge-queue=20enqueue=20guar?= =?UTF-8?q?d=20was=20not=20debt=20=E2=80=94=20the=20code=20it=20guarded=20?= =?UTF-8?q?had=20no=20callers=20(#3205)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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. --- packages/core/src/store.ts | 13 +-- .../core/src/task-store/merge-queue-ops-2.ts | 99 ++----------------- packages/core/src/task-store/moves.ts | 4 +- .../core/src/task-store/task-artifacts-ops.ts | 2 +- .../lib/lifecycle-column-census-baseline.json | 1 - 5 files changed, 17 insertions(+), 102 deletions(-) diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index 63b3f4e8e1..14798d12ee 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -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 { /* 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 { 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); } diff --git a/packages/core/src/task-store/merge-queue-ops-2.ts b/packages/core/src/task-store/merge-queue-ops-2.ts index b6f49bf197..e733c42491 100644 --- a/packages/core/src/task-store/merge-queue-ops-2.ts +++ b/packages/core/src/task-store/merge-queue-ops-2.ts @@ -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 { const layer = store.asyncLayer!; return releaseMergeQueueLeaseAsync(layer, taskId, workerId, outcome); diff --git a/packages/core/src/task-store/moves.ts b/packages/core/src/task-store/moves.ts index c3596b82b3..892020623f 100644 --- a/packages/core/src/task-store/moves.ts +++ b/packages/core/src/task-store/moves.ts @@ -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. diff --git a/packages/core/src/task-store/task-artifacts-ops.ts b/packages/core/src/task-store/task-artifacts-ops.ts index 200d3e8829..e81f1e8b78 100644 --- a/packages/core/src/task-store/task-artifacts-ops.ts +++ b/packages/core/src/task-store/task-artifacts-ops.ts @@ -88,7 +88,7 @@ export async function recordPluginActivationImpl(store: TaskStore, input: Plugin export async function enqueueMergeQueueImpl(store: TaskStore, taskId: string, opts: MergeQueueEnqueueOptions = {}): Promise { /* 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: diff --git a/scripts/lib/lifecycle-column-census-baseline.json b/scripts/lib/lifecycle-column-census-baseline.json index 1e0976bd49..e3f2fa25ac 100644 --- a/scripts/lib/lifecycle-column-census-baseline.json +++ b/scripts/lib/lifecycle-column-census-baseline.json @@ -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,