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

fleet: store.ts 12 → 11 + names the sync-dependency-loop class blocking ~10 sites across 3 clusters (#2709)

Browse Source 
Claiming **`packages/core/src/store.ts`** (12). One conversion and a
triage — because **10 of the 12 share a single blocking shape** that is
worth naming once rather than rediscovering per file.

## Census before/after

| | before | after |
|---|---:|---:|
| `store.ts` column guards | **12** | **11** |

Baseline re-recorded; `--strict` exits 0.

## Converted: 1

**1386** — the in-review guard inside `withTaskLock(id, async () => …)`.
Already async, and `this` **is** the store, so
`resolveTaskLifecycleColumns(this, task.id)` resolves the review role
with `in-review` as the fallback. Import added; nothing else in the
method changes.

## The blocking class — 6 sites, and it is not specific to this file

**1772, 1791 ×2, 1874 ×2, 1916, 1917, 1933** all read **another task's**
column — a dependency's, a blocker's, an overlap candidate's — inside
**synchronous callbacks over a prefetched `taskById` map**:

```ts
const unresolvedDeps = (task.dependencies ?? []).filter((depId) => {
  const dep = taskById.get(depId);
  return dep && !dep.deletedAt && dep.column !== "done" && dep.column !== "archived";
});
```

This is not a substitution. Each dependency may belong to a **different
workflow**, so the role must be resolved *per dep* — N async resolutions
inside a sync `filter`, on a path that deliberately prefetches into a
map precisely to avoid per-item I/O.

Two honest options:

1. **Prefetch lifecycle columns alongside `taskById`** and pass a
resolved map into these predicates. Keeps them synchronous, one
resolution per distinct workflow rather than per dep. This is the one
I'd argue for.
2. Accept per-dep resolution and make the callbacks async — changes the
shape of dependency evaluation.

Both are design changes with real cost, so this is flagged rather than
guessed.

**The same shape appears in at least two other clusters I've worked**:
`TaskDetailModal`'s `overlapBlockerTask.column` (#2696) and
`register-task-workflow-routes`' dependency-summary pair (#2700), both
flagged for this exact reason. **Worth one decision covering all three**
rather than three separate judgement calls by three workers.

## Also flagged: 3

**1610** and **1739** — enclosing-scope async-ness and store access not
established at those points, so not guessed. **1933** belongs to the
sync-filter family above.

## Verification

`pnpm test:gate` **GREEN** (158 + 487 + 10 + 71) · `pnpm lint` clean ·
core `tsc` clean · `--strict` exits 0.

🤖 Generated with [Claude Code](https://claude.com/claude-code)


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Updated failed pre-merge review bypass validation to support custom
workflow boards.
* Tasks can now bypass the step when placed in the board’s configured
review lane.
* Improved error messages to identify the correct review column when
bypassing is not allowed.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-30 07:36:00 -07:00
committed by GitHub
parent ceca08b1c3
commit b0b9d1b373
3 changed files with 113 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

76
packages/core/src/__tests__/store-bypass-review.test.ts
View File

@@ -182,4 +182,80 @@ pgDescribe("TaskStore.bypassFailedPreMergeReviewStep", () => {
expect(latestFailedPreMergeStep(updated)).toBeUndefined();
});
/*
FNXC:WorkflowLifecycleColumns 2026-07-30-01:10 (PR #2709 review — greptile):
THE REJECTION MUST NAME THE COLUMN THE CHECK USED. The guard was converted to the resolved review
lane while the message still said `in-review`, so on a custom board an operator was refused and
then told to move the card to a column their board does not have — through both the CLI and the
dashboard, with nothing in the error to reveal the real target.
That is worse than an unconverted guard. An inert guard fails visibly; this one refuses CORRECTLY
and then misdirects, so the operator's next three attempts are all wrong for a reason the product
told them.
*/
it("accepts a humanReview-ONLY lane, which the singular `.review` excluded", async () => {
/*
FNXC:WorkflowLifecycleColumns 2026-07-30-16:05 (PR #2718 review — greptile):
`.review` is the single `mergeOrchestration` column, so a board hosting review on a `humanReview`-
only lane failed this guard — `TaskContextMenu` offered "Bypass failed review" (it asks by ROLE)
and the store refused it. The operator's only escape from a stranded failed pre-merge step returned
a conflict.
The BROAD set is right here because this guard refuses or permits and moves nothing; #2750
documents why a caller that admits and then MOVES wants the narrow lane instead.
*/
const definition = await store().createWorkflowDefinition({
name: "human-review-bypass",
ir: {
version: "v2",
name: "human-review-bypass",
columns: [
{ id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold" }] },
{ id: "building", name: "Building", traits: [{ trait: "wip" }] },
{ id: "signoff", name: "Sign-off", traits: [{ trait: "human-review" }] },
{ id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
],
nodes: [{ id: "start", kind: "start", column: "backlog" }, { id: "end", kind: "end", column: "shipped" }],
edges: [{ from: "start", to: "end" }],
},
} as never);
await store().createTaskWithReservedId(
{ description: "human-review bypass", column: "signoff", workflowId: definition.id } as never,
{ taskId: "FN-HRB", applyDefaultWorkflowSteps: false },
);
await store().updateTask("FN-HRB", { workflowStepResults: [failedStep()] });
/* Passes the lane guard; any later refusal is a different gate, which is the point. */
await expect(
store().bypassFailedPreMergeReviewStep("FN-HRB", { reason: "operator override" } as never),
).resolves.toBeDefined();
});
it("names the board's OWN review column when refusing a card that is elsewhere", async () => {
const definition = await store().createWorkflowDefinition({
name: "renamed-review",
ir: {
version: "v2",
name: "renamed-review",
columns: [
{ id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold" }] },
{ id: "building", name: "Building", traits: [{ trait: "wip" }] },
{ id: "validating", name: "Validating", traits: [{ trait: "merge" }] },
{ id: "shipped", name: "Shipped", traits: [{ trait: "complete" }] },
],
nodes: [{ id: "start", kind: "start", column: "backlog" }, { id: "end", kind: "end", column: "shipped" }],
edges: [{ from: "start", to: "end" }],
},
} as never);
await store().createTaskWithReservedId(
{ description: "renamed board card", column: "building", workflowId: definition.id } as never,
{ taskId: "FN-RENAMED", applyDefaultWorkflowSteps: false },
);
await expect(
store().bypassFailedPreMergeReviewStep("FN-RENAMED", { reason: "operator override" } as never),
).rejects.toThrow(/must be in 'validating'/);
});
});

40
packages/core/src/store.ts
View File

@@ -126,6 +126,8 @@ 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 } from "./workflow-lifecycle-traits.js";
import { resolveWorkflowIrForTask } from "./workflow-ir-resolver.js";
// FNXC:RuntimeBackendAsync 2026-06-24-10:15:
// Async helper imports for backend-mode (AsyncDataLayer/PostgreSQL) delegation.
// persistence/allocator/settings/search/lifecycle/merge/archive helpers preserve
@@ -749,7 +751,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
return reconcileStaleSymbolLocksAsync(this);
}
/** FNXC:SymbolLock 2026-07-31-10:00: FN-8306 resolves only durable task declarations; PROMPT is never re-read here. */
/** FNXC:SymbolLock 2026-07-30-10:00: FN-8306 resolves only durable task declarations; PROMPT is never re-read here. */
async resolveTaskSymbols(taskId: string): Promise<TaskSymbolResolution> {
try {
return resolveTaskSymbolsForTask(await this.getTask(taskId));
@@ -1383,8 +1385,38 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
const dir = this.taskDir(id);
const task = await this.readTaskJson(dir);
if (task.column !== "in-review") {
throw new Error(`Cannot bypass review lane for ${id}: task is in '${task.column}', must be in 'in-review'`);
/*
FNXC:WorkflowLifecycleColumns 2026-07-30-01:10 (PR #2709 review — greptile):
THE MESSAGE MUST NAME THE COLUMN THE CHECK ACTUALLY USED. The guard was converted to the
resolved review lane while the rejection still said `in-review`, so on a custom board an
operator was told to move the card to a column their board does not have — through both the
CLI and the dashboard, with no way to discover the real answer from the error.
Wrong guidance is worse than an unconverted guard: an inert guard fails visibly, while this
one refuses correctly and then sends the operator somewhere that does not exist. Resolved once
into a local so the check and the message cannot drift apart again.
*/
/*
FNXC:WorkflowLifecycleColumns 2026-07-30-16:05 (PR #2718 review — greptile, on the guard I
converted in #2709):
EVERY REVIEW LANE, because `.review` is the single `mergeOrchestration` column. A board hosting
review on a `humanReview`- or `mergeBlocker`-only lane failed this check, so `TaskContextMenu`
offered "Bypass failed review" (it asks by ROLE) and the store refused it — the operator's only
escape from a stranded failed pre-merge step returned a conflict.
THE BROAD SET IS RIGHT HERE, and that is a decision rather than a default: this guard REFUSES or
PERMITS an operator action and moves nothing, so admitting every lane where review happens cannot
send a card anywhere the engine disagrees with. #2750 documents the split — a caller that admits
and then MOVES wants the narrow single lane instead.
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.
*/
const reviewIr = await resolveWorkflowIrForTask(this, task.id).catch(() => undefined);
const reviewColumns = reviewIr === undefined ? ["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}`);
}
if (task.paused) {
throw new Error(`Cannot bypass review lane for ${id}: task is paused`);
@@ -2399,7 +2431,7 @@ Issue #2149 requires read-only type filtering to occur in the file-store before
the three U5 reconciliation guards (now unconditional) and the three v1-IR
rollback-compat persistence sites (now unconditional, same stored bytes).
FNXC:WorkflowColumns 2026-07-31-04:00 (U12): `isWorkflowColumnsCompatibilityFlagEnabled` is now
FNXC:WorkflowColumns 2026-07-30-04:00 (U12): `isWorkflowColumnsCompatibilityFlagEnabled` is now
DELETED TOO. Its last two readers were the move path — `moves.ts` and the preflight in
`workflow-task-create-ops.ts` — and both were un-gated in one commit because the preflight computes
what `moves.ts` consumes.

2
scripts/lib/lifecycle-column-census-baseline.json
View File

@@ -3,8 +3,8 @@
"byFile": {
"packages/engine/src/self-healing.ts": 110,
"packages/engine/src/executor.ts": 15,
"packages/core/src/store.ts": 12,
"packages/engine/src/scheduler.ts": 12,
"packages/core/src/store.ts": 11,
"packages/core/src/task-store/async-comments-attachments.ts": 9,
"packages/dashboard/app/components/TaskContextMenu.tsx": 9,
"packages/engine/src/notification/notification-service.ts": 9,