fleet: default-workflow-hooks.ts 7 → 0 — every duration display read ZERO on a renamed board (#2734)
Claiming `default-workflow-hooks.ts` (7 → **0**), verified free against every open PR's diff first. ## Not a vocabulary tidy — three silent zeroes This file's header names it for the default workflow, but the store runs it on the flag-ON path for **every** workflow: the trait registry resolves each hook by **trait id**, not by workflow. `reopen-semantics-by-role.test.ts` already documents that exact hazard for the reopen predicates. The **timing, completion and in-review hooks had the same defect** and were not part of that conversion. On a renamed board, with nothing thrown and nothing logged: - **`applyTimingEffects`** accrues `cumulativeActiveMs` while a card sits in the WIP lane. With the lane named, the exit test never fires — so **no active time is ever accrued**, and `productivity-analytics.ts`, `task-timing.ts` and every duration display read **zero**. - **`applyCompletionTimingEffects`** never stamps `executionCompletedAt`, so a finished card looks unfinished to anything reading that field. - **`applyInReviewEnterEffects`** returns early, leaving the recovery counters set. The file already had the idiom — `ctx.lifecycleColumns`, `planningColumnsOf`, `liveWorkColumnsOf` with `LEGACY_` fallbacks — so this adds no abstraction. One deliberate detail: `applyTimingEffects` resolves the WIP lane **once into a local** rather than reading it twice. The exit test and the re-entry test have to agree about which column is WIP, or a rename makes the accounting count an interval twice, or not at all. ## A test that would have lied to me I wrote the new cases through `applyDefaultWorkflowMoveEffects` first, and **all three failed on the DEFAULT lineage too**. The dispatcher resolves hooks by trait, and neither test IR declares the `timing` trait, so those hooks never ran at all. That failure looks exactly like a conversion bug. Going through the dispatcher would have been testing the trait registry's wiring rather than this change — so the cases call the converted functions directly, and the reason is recorded in the test. ## Revert proof — all three, each naming the renamed lineage | reverted | failure | |---|---| | the `in-progress` literals | `renamed lineage accrued no active time: expected undefined to be 300000` | | the `done` literal | `renamed lineage did not stamp completion: expected undefined to be '2026-07-30T00:00:00.000Z'` | | the `in-review` literal | `renamed lineage kept its recovery counter: expected 3 to be undefined` | Every case runs on **both** lineages and the default one passes either way — which is the point of running it. ## A finding I did not act on **`evaluateMergeBlockerGuard` appears exactly once in the repo — its own definition.** And the file header says it is "implemented as the `evaluateDefaultWorkflowGuards` reader", which does not exist either. The merge-blocker guard hook is **defined and never consulted**. I converted it (trailing optional lifecycle param, matching `DefaultWorkflowMoveContext`) but did not delete it: the header states this file is a deliberate parallel of `store.ts`'s flag-off path so the two can be parity-checked, which makes removing it a scope call for whoever owns that convergence — not something to decide inside a conversion. ## Verification `pnpm test:gate` **GREEN** (158 + 10 + 487 + 71) · **22 passed** across `default-workflow-hooks` + `reopen-semantics-by-role` · core `tsc` clean · `pnpm lint` clean · census `--strict` exits 0. 🤖 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
@@ -0,0 +1,97 @@
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-15:10 (PR #2734 review — greptile, on my own code):
|
||||
|
||||
THE PRODUCER IS THE THING UNDER TEST, not the hook.
|
||||
|
||||
`moves.ts` builds `lifecycleColumnSets.review` and hands it to the default-workflow hooks. It built
|
||||
that set from `columnsWithFlag(ir, "mergeOrchestration")` alone, so a workflow hosting review on a
|
||||
`humanReview`-only lane produced an EMPTY review set — and `applyInReviewEnterEffects` then never ran
|
||||
for a card plainly in review, leaving the recovery counters it clears set.
|
||||
|
||||
WHY THIS FILE EXISTS RATHER THAN A UNIT CASE. My first attempt asserted the hook directly, passing
|
||||
`lifecycleColumnSets: { review: ["signoff"] }` by hand. That exercises the hook — which was already
|
||||
correct — and passes with the producer's bug fully in place. Reverting the producer left it green.
|
||||
Only driving a real move through a real store reaches the code that was wrong.
|
||||
|
||||
LANE. `.pg.test.ts`, skipped by `pgDescribe` when no PostgreSQL is reachable, so the merge gate is
|
||||
unaffected. Throwaway per-file database; never port 4040.
|
||||
*/
|
||||
import { beforeAll, beforeEach, afterEach, afterAll, expect, it } from "vitest";
|
||||
import "@fusion/core"; // registers the built-in column traits
|
||||
|
||||
import { pgDescribe, createSharedPgTaskStoreTestHarness } from "../../__test-utils__/pg-test-harness.js";
|
||||
|
||||
pgDescribe("moves.ts supplies EVERY review lane to the lifecycle hooks", () => {
|
||||
const harness = createSharedPgTaskStoreTestHarness({ prefix: "fusion_moves_review_set" });
|
||||
|
||||
beforeAll(harness.beforeAll);
|
||||
afterAll(harness.afterAll);
|
||||
beforeEach(async () => { await harness.beforeEach(); });
|
||||
afterEach(async () => { await harness.afterEach(); });
|
||||
|
||||
it("runs the in-review enter effects for a humanReview-ONLY lane", async () => {
|
||||
const store = harness.store();
|
||||
const definition = await store.createWorkflowDefinition({
|
||||
name: "human-review-only",
|
||||
ir: {
|
||||
version: "v2",
|
||||
name: "human-review-only",
|
||||
columns: [
|
||||
{ id: "backlog", name: "Backlog", traits: [{ trait: "intake" }, { trait: "hold" }] },
|
||||
{ id: "building", name: "Building", traits: [{ trait: "wip" }] },
|
||||
/* No `merge` trait: review is hosted by human-review alone. */
|
||||
{ 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);
|
||||
|
||||
const task = await store.createTask({ description: "human-review card", workflowId: definition.id } as never);
|
||||
await store.moveTask(task.id, "building" as never, { bypassGuards: true } as never);
|
||||
|
||||
/*
|
||||
The witness: `applyInReviewEnterEffects` clears `recoveryRetryCount`. Seed it so its absence after
|
||||
the move can only mean the enter-effects ran.
|
||||
*/
|
||||
await store.updateTask(task.id, { recoveryRetryCount: 3 } as never);
|
||||
store.taskCache.delete(task.id);
|
||||
expect((await store.getTask(task.id)).recoveryRetryCount).toBe(3);
|
||||
|
||||
await store.moveTask(task.id, "signoff" as never, { bypassGuards: true } as never);
|
||||
|
||||
store.taskCache.delete(task.id);
|
||||
const moved = await store.getTask(task.id);
|
||||
expect(moved.column).toBe("signoff");
|
||||
expect(moved.recoveryRetryCount).toBeUndefined();
|
||||
});
|
||||
|
||||
it("does NOT run them entering a lane that carries no review trait", async () => {
|
||||
/* The negative half: widening the set must not make every column a review lane. */
|
||||
const store = harness.store();
|
||||
const definition = await store.createWorkflowDefinition({
|
||||
name: "human-review-only-neg",
|
||||
ir: {
|
||||
version: "v2",
|
||||
name: "human-review-only-neg",
|
||||
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);
|
||||
|
||||
const task = await store.createTask({ description: "non-review move", workflowId: definition.id } as never);
|
||||
await store.updateTask(task.id, { recoveryRetryCount: 3 } as never);
|
||||
|
||||
await store.moveTask(task.id, "building" as never, { bypassGuards: true } as never);
|
||||
|
||||
store.taskCache.delete(task.id);
|
||||
expect((await store.getTask(task.id)).recoveryRetryCount).toBe(3);
|
||||
});
|
||||
});
|
||||
@@ -30,7 +30,10 @@ import { __resetTraitRegistryForTests } from "../trait-registry.js";
|
||||
import { registerBuiltinTraits } from "../builtin-traits.js";
|
||||
import {
|
||||
__resetDefaultWorkflowHooksForTests,
|
||||
applyCompletionTimingEffects,
|
||||
applyDefaultWorkflowMoveEffects,
|
||||
applyInReviewEnterEffects,
|
||||
applyTimingEffects,
|
||||
isReopenIntoPlanning,
|
||||
registerDefaultWorkflowHooks,
|
||||
type DefaultWorkflowMoveContext,
|
||||
@@ -241,3 +244,200 @@ describe("the store's reopen check and the hooks' cannot disagree", () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-04:40 (fleet phase — the same safety argument, applied to TIMING):
|
||||
The header of this file explains why the reopen predicates had to stop naming the default lineage: these
|
||||
hooks run on the flag-ON path for EVERY workflow, because the trait registry resolves each hook by trait
|
||||
id, not by workflow. The timing, completion and in-review hooks had the same defect and were not part of
|
||||
that conversion.
|
||||
|
||||
What it costs on a renamed board, none of it throwing:
|
||||
- `applyTimingEffects` accrues `cumulativeActiveMs` while a card sits in the WIP lane. With the lane
|
||||
named, the exit test never fires, so NO active time is accrued and every duration display —
|
||||
`productivity-analytics.ts`, `task-timing.ts` — reads zero.
|
||||
- `applyCompletionTimingEffects` never stamps `executionCompletedAt`, so a finished card looks
|
||||
unfinished to anything reading that field.
|
||||
- `applyInReviewEnterEffects` returns early, so the recovery counters it clears stay set.
|
||||
|
||||
THE HOOKS ARE CALLED DIRECTLY here, not through `applyDefaultWorkflowMoveEffects`. I wrote it through the
|
||||
dispatcher first and all three cases failed on the DEFAULT lineage too: the dispatcher resolves hooks by
|
||||
TRAIT, and neither test IR declares the `timing` trait, so those hooks never ran at all. Going through the
|
||||
dispatcher would have tested the trait registry's wiring, not this conversion — and would have looked like
|
||||
a conversion bug.
|
||||
|
||||
REVERT CHECK, measured (all three run): restoring the `in-progress` literals leaves `cumulativeActiveMs`
|
||||
undefined on the renamed board; restoring the `done` literal leaves `executionCompletedAt` unset;
|
||||
restoring the `in-review` literal leaves `recoveryRetryCount` at 3. Every case runs on BOTH lineages, and
|
||||
the default one passes either way — which is the point of running it.
|
||||
*/
|
||||
describe("timing, completion and in-review effects are keyed on ROLES", () => {
|
||||
const LINEAGES = [
|
||||
{ label: "default", ir: DEFAULT_IR, wip: "in-progress", review: "in-review", complete: "done" },
|
||||
{ label: "renamed", ir: RENAMED_IR, wip: "building", review: "checking", complete: "shipped" },
|
||||
] as const;
|
||||
|
||||
it("accrues active time leaving the WIP lane on both lineages", () => {
|
||||
for (const { label, ir, wip, review } of LINEAGES) {
|
||||
const ctx = makeCtx(ir, wip, review, {
|
||||
task: {
|
||||
id: "FN-2",
|
||||
column: review,
|
||||
columnMovedAt: "2026-07-30T00:05:00.000Z",
|
||||
executionStartedAt: "2026-07-30T00:00:00.000Z",
|
||||
steps: [],
|
||||
dependencies: [],
|
||||
workflowStepResults: [],
|
||||
} as unknown as Task,
|
||||
});
|
||||
applyTimingEffects(ctx);
|
||||
expect(ctx.task.cumulativeActiveMs, `${label} lineage accrued no active time`).toBe(5 * 60_000);
|
||||
}
|
||||
});
|
||||
|
||||
it("stamps executionCompletedAt on entry to the complete lane on both lineages", () => {
|
||||
for (const { label, ir, review, complete } of LINEAGES) {
|
||||
const ctx = makeCtx(ir, review, complete);
|
||||
applyCompletionTimingEffects(ctx);
|
||||
expect(ctx.task.executionCompletedAt, `${label} lineage did not stamp completion`).toBe(
|
||||
ctx.task.columnMovedAt,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it("clears the recovery counters on entry to the review lane on both lineages", () => {
|
||||
for (const { label, ir, wip, review } of LINEAGES) {
|
||||
const ctx = makeCtx(ir, wip, review, {
|
||||
task: {
|
||||
id: "FN-3",
|
||||
column: review,
|
||||
columnMovedAt: "2026-07-30T00:00:00.000Z",
|
||||
recoveryRetryCount: 3,
|
||||
steps: [],
|
||||
dependencies: [],
|
||||
workflowStepResults: [],
|
||||
} as unknown as Task,
|
||||
});
|
||||
applyInReviewEnterEffects(ctx);
|
||||
expect(ctx.task.recoveryRetryCount, `${label} lineage kept its recovery counter`).toBeUndefined();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-09:05 (PR #2734 review — greptile):
|
||||
SECONDARY LANES. `LifecycleColumns` names one column per role by design (#2721 pinned that), so a
|
||||
workflow declaring `countsTowardWip` on two columns had a second WIP lane the hooks did not recognise.
|
||||
|
||||
The timing consequence is the concrete one: a move BETWEEN two WIP lanes looked like an exit from WIP
|
||||
followed by a re-entry, so `cumulativeActiveMs` closed and reopened a segment the card never left.
|
||||
*/
|
||||
describe("timing hooks honour EVERY lane carrying the role", () => {
|
||||
const ctx = (fromColumn: string, toColumn: string, sets?: { wip?: readonly string[] }) => {
|
||||
const task = {
|
||||
id: "FN-1",
|
||||
columnMovedAt: "2026-07-30T10:00:00.000Z",
|
||||
executionStartedAt: "2026-07-30T09:00:00.000Z",
|
||||
cumulativeActiveMs: 0,
|
||||
} as never as { cumulativeActiveMs?: number };
|
||||
return {
|
||||
task,
|
||||
ctx: {
|
||||
task,
|
||||
fromColumn,
|
||||
toColumn,
|
||||
movedAt: "2026-07-30T10:00:00.000Z",
|
||||
lifecycleColumns: { wip: "building", complete: "shipped" },
|
||||
lifecycleColumnSets: sets,
|
||||
resetSteps: () => {},
|
||||
} as never,
|
||||
};
|
||||
};
|
||||
|
||||
it("does NOT close the active segment when a card moves between two WIP lanes", () => {
|
||||
const { task, ctx: c } = ctx("building", "building-two", { wip: ["building", "building-two"] });
|
||||
|
||||
applyTimingEffects(c);
|
||||
|
||||
// Still inside WIP, so no segment was accrued on the way out.
|
||||
expect(task.cumulativeActiveMs).toBe(0);
|
||||
});
|
||||
|
||||
it("DOES close it when the card genuinely leaves every WIP lane", () => {
|
||||
const { task, ctx: c } = ctx("building", "signoff", { wip: ["building", "building-two"] });
|
||||
|
||||
applyTimingEffects(c);
|
||||
|
||||
expect(task.cumulativeActiveMs).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it("runs the review enter-effects for a humanReview-ONLY lane", () => {
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-15:10 (PR #2734 review — greptile, on my own code):
|
||||
The producer built this set from `mergeOrchestration` alone, so a workflow hosting review on a
|
||||
`humanReview`- or `mergeBlocker`-only lane skipped `applyInReviewEnterEffects` entirely — the
|
||||
recovery counters it clears stayed set for a card plainly in review.
|
||||
|
||||
Fixed by using core's `resolveReviewColumns` (#2730) rather than a fifth inline union. That is the
|
||||
BROAD set, which is right here: these hooks ASK the question and move nothing on the answer. A
|
||||
caller that admits and then MOVES wants the narrow single lane — #2750 documents the split.
|
||||
*/
|
||||
const task = {
|
||||
id: "FN-HR",
|
||||
column: "signoff",
|
||||
columnMovedAt: "2026-07-30T00:00:00.000Z",
|
||||
recoveryRetryCount: 3,
|
||||
steps: [],
|
||||
dependencies: [],
|
||||
workflowStepResults: [],
|
||||
} as unknown as Task;
|
||||
|
||||
applyInReviewEnterEffects({
|
||||
task,
|
||||
fromColumn: "building",
|
||||
toColumn: "signoff",
|
||||
movedAt: "2026-07-30T00:00:00.000Z",
|
||||
lifecycleColumns: { wip: "building" },
|
||||
lifecycleColumnSets: { review: ["signoff"] },
|
||||
resetSteps: () => {},
|
||||
} as never);
|
||||
|
||||
expect(task.recoveryRetryCount).toBeUndefined();
|
||||
});
|
||||
|
||||
it("treats an EMPTY set as 'no lane carries this role', not as a missing answer", () => {
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-10:40 (PR #2734 review — greptile):
|
||||
`lifecycleColumnSets` is populated only when the caller resolved an IR, so an empty array means the
|
||||
board was READ and declares no WIP lane. The first version guarded on `length > 0` and fell back to
|
||||
the singular id and then the legacy name, so a traitless column merely NAMED `in-progress` accrued
|
||||
timing as though it were the WIP lane.
|
||||
|
||||
Undefined means "could not read"; empty means "read, and the answer is none". Same distinction as
|
||||
#2731's `?? {}` and #2733's refusal to invent a complete column — which I applied in both of those
|
||||
and then got wrong here.
|
||||
*/
|
||||
const { task, ctx: c } = ctx("in-progress", "signoff", { wip: [] });
|
||||
/*
|
||||
The singular role must be ABSENT for this to discriminate. With `lifecycleColumns.wip` set, the
|
||||
buggy fallback lands on that name and answers false anyway — my first version of this case did
|
||||
exactly that and passed with the defect in place. Only when the singular is absent does the bug
|
||||
reach the LEGACY name and treat a traitless `in-progress` column as WIP.
|
||||
*/
|
||||
(c as unknown as { lifecycleColumns: Record<string, unknown> }).lifecycleColumns = { complete: "shipped" };
|
||||
|
||||
applyTimingEffects(c);
|
||||
|
||||
// No WIP lane exists, so leaving `in-progress` is not leaving WIP and nothing accrues.
|
||||
expect(task.cumulativeActiveMs).toBe(0);
|
||||
});
|
||||
|
||||
it("falls back to the singular role when no set is supplied", () => {
|
||||
/* Additive: a caller that does not pass sets keeps exactly the previous behaviour. */
|
||||
const { task, ctx: c } = ctx("building", "signoff");
|
||||
|
||||
applyTimingEffects(c);
|
||||
|
||||
expect(task.cumulativeActiveMs).toBeGreaterThan(0);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -56,8 +56,19 @@ export function evaluateMergeBlockerGuard(
|
||||
task: Pick<Task, "column" | "paused" | "status" | "error" | "steps" | "workflowStepResults">,
|
||||
fromColumn: string,
|
||||
toColumn: string,
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-04:20 (fleet phase):
|
||||
The moving task's resolved lifecycle columns, so the review -> complete crossing this guard fires on
|
||||
is a ROLE crossing. OPTIONAL and last, matching `DefaultWorkflowMoveContext.lifecycleColumns`: absent,
|
||||
the legacy ids answer, which is the same degraded contract `planningColumnsOf` and
|
||||
`liveWorkColumnsOf` already use in this file.
|
||||
*/
|
||||
lifecycleColumns?: LifecycleColumns | undefined,
|
||||
): GuardVerdict {
|
||||
if (fromColumn === "in-review" && toColumn === "done") {
|
||||
if (
|
||||
fromColumn === (lifecycleColumns?.review ?? "in-review")
|
||||
&& toColumn === (lifecycleColumns?.complete ?? "done")
|
||||
) {
|
||||
return getTaskMergeBlocker(task);
|
||||
}
|
||||
return undefined;
|
||||
@@ -116,10 +127,51 @@ export interface DefaultWorkflowMoveContext {
|
||||
* only in that no-basis case, never as a substitute for an absent role.
|
||||
*/
|
||||
lifecycleColumns?: LifecycleColumns | undefined;
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-09:05 (PR #2734 review — greptile):
|
||||
THE SET-SHAPED COMPANION, because `LifecycleColumns` names ONE column per role by design — #2721
|
||||
pinned that as the contract, not a gap. A workflow may put `countsTowardWip` (or `complete`, or
|
||||
`mergeOrchestration`) on several columns, and these hooks ask "is this card IN that role", which is
|
||||
membership.
|
||||
|
||||
Concretely for the timing hook: with a single id, a card moving between two WIP lanes looked like an
|
||||
EXIT from WIP followed by a re-entry, so `cumulativeActiveMs` closed and reopened a segment the card
|
||||
never left — and a card living only in the secondary lane accrued nothing at all.
|
||||
|
||||
Optional and additive: absent, every read falls back to the singular struct and then the legacy id,
|
||||
so nothing changes for the default lineage or for a caller that does not supply it. The caller in
|
||||
`moves.ts` already holds the IR, so populating it costs no extra read.
|
||||
*/
|
||||
lifecycleColumnSets?: { wip?: readonly string[]; complete?: readonly string[]; review?: readonly string[] } | undefined;
|
||||
/** Reset all steps to pending + currentStep 0 (store owns the impl). */
|
||||
resetSteps: () => void;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-10:40 (PR #2734 review — greptile, and it is the same
|
||||
distinction I had just applied twice elsewhere and then got wrong here):
|
||||
AN EMPTY SET IS AN ANSWER, NOT AN ABSENT ONE.
|
||||
|
||||
`lifecycleColumnSets` is supplied only when the caller resolved a workflow IR, so `set !== undefined`
|
||||
already means "the board was read". An EMPTY array then says "no column carries this role" — and the
|
||||
`length > 0` guard treated that as no-basis and fell back to the singular id, then to the legacy name.
|
||||
The consequence: on a valid v2 workflow with no WIP lane, a traitless column happening to be NAMED
|
||||
`in-progress` accrued timing as though it were one.
|
||||
|
||||
Same shape as #2731 (`?? {}` for resolved-but-roleless flags) and #2733 (refuse rather than invent a
|
||||
complete column). Undefined means "could not read"; empty means "read, and the answer is none".
|
||||
*/
|
||||
/** Membership for a role: the resolved SET when the caller supplied one, else the singular id, else the legacy id. */
|
||||
function inRole(
|
||||
column: string,
|
||||
set: readonly string[] | undefined,
|
||||
single: string | undefined,
|
||||
legacy: string,
|
||||
): boolean {
|
||||
if (set !== undefined) return set.includes(column);
|
||||
return column === (single ?? legacy);
|
||||
}
|
||||
|
||||
// ── Field-mutation effects (applied in-lock, before commit) ───────────────────
|
||||
//
|
||||
// These mirror the inline flag-off mutations in store.ts exactly. They run as
|
||||
@@ -145,7 +197,16 @@ export interface DefaultWorkflowMoveContext {
|
||||
*/
|
||||
export function applyTimingEffects(ctx: DefaultWorkflowMoveContext): void {
|
||||
const { task, fromColumn, toColumn } = ctx;
|
||||
if (fromColumn === "in-progress" && toColumn !== "in-progress") {
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-04:20 (fleet phase):
|
||||
`cumulativeActiveMs` accrues while a card sits in the WIP lane, so both halves of the segment
|
||||
boundary must name that lane by ROLE. Keyed on one resolved value rather than two independent reads:
|
||||
the exit test and the re-entry test have to agree about which column is WIP or a rename makes the
|
||||
accounting count the same interval twice, or not at all.
|
||||
*/
|
||||
const isWip = (column: string) =>
|
||||
inRole(column, ctx.lifecycleColumnSets?.wip, ctx.lifecycleColumns?.wip, "in-progress");
|
||||
if (isWip(fromColumn) && !isWip(toColumn)) {
|
||||
const segmentStartMs = Date.parse(task.executionStartedAt ?? task.columnMovedAt ?? ctx.movedAt);
|
||||
const segmentEndMs = Date.parse(task.columnMovedAt ?? ctx.movedAt);
|
||||
const segmentDeltaMs =
|
||||
@@ -154,7 +215,7 @@ export function applyTimingEffects(ctx: DefaultWorkflowMoveContext): void {
|
||||
: 0;
|
||||
task.cumulativeActiveMs = Math.max(0, task.cumulativeActiveMs ?? 0) + segmentDeltaMs;
|
||||
}
|
||||
if (toColumn === "in-progress") {
|
||||
if (isWip(toColumn)) {
|
||||
task.cumulativeActiveMs ??= 0;
|
||||
if (!task.firstExecutionAt) task.firstExecutionAt = task.columnMovedAt;
|
||||
if (!task.executionStartedAt) task.executionStartedAt = task.columnMovedAt;
|
||||
@@ -165,7 +226,7 @@ export function applyTimingEffects(ctx: DefaultWorkflowMoveContext): void {
|
||||
/** Stamp `executionCompletedAt` on entry to a completion column. */
|
||||
export function applyCompletionTimingEffects(ctx: DefaultWorkflowMoveContext): void {
|
||||
const { task, toColumn } = ctx;
|
||||
if (toColumn === "done" && !task.executionCompletedAt) {
|
||||
if (inRole(toColumn, ctx.lifecycleColumnSets?.complete, ctx.lifecycleColumns?.complete, "done") && !task.executionCompletedAt) {
|
||||
task.executionCompletedAt = task.columnMovedAt;
|
||||
}
|
||||
}
|
||||
@@ -298,7 +359,7 @@ export function isReopenIntoPlanning(
|
||||
* block in store.ts. */
|
||||
export function applyInReviewEnterEffects(ctx: DefaultWorkflowMoveContext): void {
|
||||
const { task, toColumn } = ctx;
|
||||
if (toColumn !== "in-review") return;
|
||||
if (!inRole(toColumn, ctx.lifecycleColumnSets?.review, ctx.lifecycleColumns?.review, "in-review")) return;
|
||||
// Do not snapshot the global autoMerge setting here. Undefined means "follow
|
||||
// the live global setting"; only an explicit task value should stay sticky.
|
||||
task.recoveryRetryCount = undefined;
|
||||
|
||||
@@ -26,7 +26,7 @@ import {
|
||||
evaluateTransitionInvariants,
|
||||
} from "../workflow-transition-policy.js";
|
||||
import {type DefaultWorkflowMoveContext, applyDefaultWorkflowMoveEffects, isReopenIntoPlanning} from "../default-workflow-hooks.js";
|
||||
import {resolveLifecycleColumns} from "../workflow-lifecycle-traits.js";
|
||||
import {columnsWithFlag, resolveLifecycleColumns, resolveReviewColumns} from "../workflow-lifecycle-traits.js";
|
||||
import {makeTransitionRejection, makeTransitionPending} from "../transition-types.js";
|
||||
import {writeTransitionPendingAsync, clearTransitionPendingAsync} from "./async-transition-pending.js";
|
||||
import type {WorkflowIr} from "../workflow-ir-types.js";
|
||||
@@ -862,6 +862,32 @@ export async function moveTaskInternalImpl(store: TaskStore, id: string, toColum
|
||||
names on every workflow, so a renamed board got no reopen effects at all.
|
||||
*/
|
||||
lifecycleColumns: moveLifecycleColumns,
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-09:05 (PR #2734 review — greptile):
|
||||
The SET-shaped roles beside the singular ones. `workflowIr` is already in hand here, so these
|
||||
cost three trait scans and no extra read — and they are what let the timing hook treat a move
|
||||
BETWEEN two WIP lanes as staying in WIP rather than as an exit plus a re-entry.
|
||||
*/
|
||||
lifecycleColumnSets: workflowIr === undefined
|
||||
? undefined
|
||||
: {
|
||||
wip: columnsWithFlag(workflowIr, "countsTowardWip"),
|
||||
complete: columnsWithFlag(workflowIr, "complete"),
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-15:10 (PR #2734 review — greptile, on my own
|
||||
code): `mergeOrchestration` ALONE was the wrong set. A workflow may host review on a
|
||||
`humanReview`- or `mergeBlocker`-only lane, and entering it then skipped
|
||||
`applyInReviewEnterEffects` entirely — the enter-effects simply did not run for a card
|
||||
plainly in review.
|
||||
|
||||
`resolveReviewColumns` (core, merged in #2730) is the shared answer to exactly this
|
||||
question, so this becomes the first consumer to use it instead of a fifth inline union.
|
||||
Note it is the BROAD set — correct here, because these hooks ASK "is this card in a review
|
||||
lane" and do not move anything on the answer. A caller that ADMITS and then MOVES wants the
|
||||
narrow single lane instead; #2750 documents that split at the helper.
|
||||
*/
|
||||
review: resolveReviewColumns(workflowIr),
|
||||
},
|
||||
};
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-08:10: the store's own copy of the reopen
|
||||
|
||||
@@ -9,7 +9,6 @@
|
||||
"packages/dashboard/app/components/TaskContextMenu.tsx": 9,
|
||||
"packages/dashboard/src/github-tracking-reconciler.ts": 9,
|
||||
"packages/engine/src/notification/notification-service.ts": 9,
|
||||
"packages/core/src/default-workflow-hooks.ts": 7,
|
||||
"packages/dashboard/app/components/Column.tsx": 7,
|
||||
"packages/core/src/live-agent-count.ts": 6,
|
||||
"packages/core/src/task-merge.ts": 6,
|
||||
|
||||
Reference in New Issue
Block a user