fix(engine,core): role resolution was half-done in two shared lifecycle predicates (surfacing family + file-scope leases) (#2975)
The three surfacing sweeps stopped reporting anything for a card resting in a board's **second** review or hold column. A lifecycle role is a **trait**, and any number of columns may carry it. The shared runner resolved it with `resolveLifecycleColumns()[role]` — **first match** — then gated on it: ```ts const roleColumn = lifecycle?.[spec.role]; // FIRST column carrying the trait if (task.column !== resolved.roleColumn) continue; // everything else dropped ``` A workflow that splits human sign-off from the merge lane has two review columns; one that parks dependency-blocked cards separately has two hold columns. Cards in the second got **no stale-paused-todo, no stale-paused-review, no in-review-stalled** diagnostic — silently, with no error, on all three sweeps at once. ## The second bug hiding inside the fix for the first Resolving membership but still reading `roleColumns[0]`'s declared `recovery` applies the **merge lane's** threshold to a card sitting in the **sign-off** lane. Each card's policy now comes from its own column, and one of the new cases fails if it doesn't: the first role column declares a policy that suppresses the signal, the card's own column declares one that fires. ## Reverted | | | |---|---| | **6 of 12** new cases fail | `fires for a card in the SECOND column carrying its role` and `reads the recovery policy of the card's OWN role column` — × 3 sweeps | | the other 6 pass either way | non-regression halves: still fires for the FIRST role column, still does **not** fire for a card outside every role column. Membership must widen the gate, not move it. | The pre-existing 45 cases were all green throughout — the single-role-column fixture could not express the case, which is why the table-driven file that exists to stop these three sweeps drifting apart never caught it. ## Verification `pnpm test:gate` 161 + 13 + 487 + 71 · surfacing family 57 · core stale-paused 20 · lint · census `--strict` · sql-literals · fnxc-dates · lane-wiring · changesets — all green. ## Note `holdColumns` was missing from the lane-wiring vocabulary, so the gate could not see that argument dropped. Added in the same commit. While reviewing, I found and measured **two problems in #2974** (comment posted there): six of its newly-visible sites are `satisfies`-wrapped false positives, and baselining them means deleting a real `reviewColumns` argument keeps the count unchanged and the gate green; and its baseline predates #2970, re-opening the slot that PR closed. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved stale-card detection across all applicable review and hold columns. * Cards are now surfaced using the policies configured for their specific lifecycle column. * Cards outside matching lifecycle columns are no longer incorrectly surfaced. * Preserved existing fallback behavior when no lifecycle columns are configured. * **Tests** * Added coverage for workflows with split review and hold columns. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- ## Second commit: the same predicate, half-converted (`shouldHoldActiveFileScopeLease`) Folded in here rather than stacked — same file, same class, and a stacked PR on an unmerged base is not mergeable. Reversible; say the word and I'll split it. `shouldHoldActiveFileScopeLease` is the **scheduler's** lease predicate, shared with the self-healing repair paths deliberately so the two cannot disagree about who holds a file-scope lease. Its two role answers are optional parameters defaulting to the legacy ids. The scheduler's own call sites were converted to pass resolved answers; self-healing's two were not: ```ts const isWipColumn = options?.isWipColumn ?? task.column === "in-progress"; const isReviewColumn = options?.isReviewColumn ?? task.column === "in-review"; ``` On a renamed board neither branch matches, so the predicate returns `false` for every card. The scheduler kept the lease; self-healing saw none, cleared `overlapBlockedBy`, and **released a dependent to edit files another agent still holds** — the outcome `groupOverlappingFiles` exists to prevent. Membership comes from the wip/review sets each sweep already resolved a few lines above, so this adds no reads. **Reverted:** both new cases fail with `overlapBlockedBy` = `null` — the release itself, not a proxy. The pre-existing legacy-column case in the same file passes either way, because `in-progress` satisfies the literal default; that is exactly why it never caught this. Lane-wiring baseline re-recorded `9 -> 7` in the same commit (the ratchet refused a stale allowance, as intended). **Verification:** gate 161 + 13 + 487 + 71 · surfacing 57 · overlap-seam + scheduler-lease + query-blindness 79 · core stale-paused 20 · lint · census `--strict` · sql-literals · fnxc-dates · changesets — green.
This commit is contained in:
gsxdsm
7
.changeset/self-healing-lease-role-answers.md
Normal file
7
.changeset/self-healing-lease-role-answers.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Stale-dependency cleanup no longer releases a task to edit files another agent still holds, on renamed boards.
|
||||
category: fix
|
||||
dev: The two `shouldHoldActiveFileScopeLease` call sites in self-healing now pass resolved `isWipColumn`/`isReviewColumn` from the wip/review sets those sweeps already resolve, matching the scheduler's own call sites.
|
||||
7
.changeset/surfacing-role-membership.md
Normal file
7
.changeset/surfacing-role-membership.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Stale-card diagnostics now cover every review and hold column, not just the first of each.
|
||||
category: fix
|
||||
dev: `runSurfacingSweep`'s role gate resolves membership (`resolveReviewColumns` / `columnsWithFlag(ir,"hold")`) instead of `resolveLifecycleColumns()[role]`; signals receive the column SET, and each card's recovery policy is read from its own column. Adds `StalePausedTodoContext.holdColumns`.
|
||||
@@ -26,6 +26,16 @@ export interface StalePausedTodoContext {
|
||||
`resolveLifecycleColumns(ir).hold` instead.
|
||||
*/
|
||||
holdColumn?: string;
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-21:40 (the lane seam, MEMBERSHIP not one column):
|
||||
`holdColumn` is `resolveLifecycleColumns().hold` — the FIRST column carrying the hold trait. A
|
||||
board declaring more than one hold lane (a capacity wait beside a blocked-on-dependency park) has
|
||||
several, and a card resting in the second read as not-on-hold and was never surfaced.
|
||||
|
||||
Mirrors `reviewColumns` in in-review-stalled.ts / stale-paused-review.ts. Optional, with today's
|
||||
behaviour preserved as the fallback, so a caller that does not pass it is byte-identical.
|
||||
*/
|
||||
holdColumns?: ReadonlySet<string>;
|
||||
now?: number;
|
||||
thresholdMs?: number;
|
||||
engineActiveSinceMs?: number;
|
||||
@@ -38,8 +48,11 @@ export function getStalePausedTodoSignal(
|
||||
task: Pick<Task, "column" | "paused" | "columnMovedAt" | "updatedAt" | "pausedReason" | "pausedByAgentId">,
|
||||
context: StalePausedTodoContext = {},
|
||||
): StalePausedTodoSignal | undefined {
|
||||
const holdColumn = context.holdColumn ?? "todo";
|
||||
if (task.column !== holdColumn || task.paused !== true) return undefined;
|
||||
const onHoldLane = context.holdColumns
|
||||
? context.holdColumns.has(task.column)
|
||||
/* DELIBERATE-LITERAL — the no-metadata fallback; a supplied set always wins. */
|
||||
: task.column === (context.holdColumn ?? "todo");
|
||||
if (!onHoldLane || task.paused !== true) return undefined;
|
||||
|
||||
const thresholdMs = context.thresholdMs ?? DEFAULT_STALE_PAUSED_TODO_THRESHOLD_MS;
|
||||
if (!Number.isFinite(thresholdMs) || thresholdMs <= 0) return undefined;
|
||||
|
||||
@@ -135,3 +135,120 @@ describe("SelfHealingManager fake TaskStore overlap seam", () => {
|
||||
manager.stop();
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-22:25 (the shared lease predicate was HALF-converted):
|
||||
`shouldHoldActiveFileScopeLease` is the scheduler's predicate, shared with self-healing on purpose so
|
||||
the two cannot disagree about who holds a file-scope lease. Its role answers are optional parameters
|
||||
defaulting to the legacy ids; the scheduler passes resolved answers and this sweep did not, so on a
|
||||
renamed board the scheduler kept a lease that this sweep saw as absent — and released a dependent to
|
||||
edit files another agent still holds.
|
||||
|
||||
The board below is renamed but otherwise identical to the legacy case above, which is the point: the
|
||||
existing test passes either way because `in-progress` satisfies the literal default.
|
||||
*/
|
||||
const RENAMED_BOARD_IR = {
|
||||
version: "v2",
|
||||
id: "custom:renamed",
|
||||
nodes: [],
|
||||
edges: [],
|
||||
columns: [
|
||||
{ id: "drafting", name: "drafting", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "building", name: "building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "checking", name: "checking", traits: [{ trait: "merge" }] },
|
||||
{ id: "shipped", name: "shipped", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
};
|
||||
|
||||
describe("SelfHealingManager stale-blocker cleanup on a RENAMED board", () => {
|
||||
function createRenamedBoardStore(seed: Task[]) {
|
||||
const tasks = new Map(seed.map((task) => [task.id, task]));
|
||||
const settings = {
|
||||
globalPause: false,
|
||||
enginePaused: false,
|
||||
mergeRequestContractShadowEnabled: true,
|
||||
} as Settings;
|
||||
const store = {
|
||||
getSettings: vi.fn().mockResolvedValue(settings),
|
||||
listTasks: vi.fn().mockImplementation(async (opts?: { column?: Task["column"] }) => {
|
||||
const all = [...tasks.values()];
|
||||
return opts?.column ? all.filter((task) => task.column === opts.column) : all;
|
||||
}),
|
||||
getTask: vi.fn().mockImplementation(async (id: string) => tasks.get(id) ?? null),
|
||||
updateTask: vi.fn().mockImplementation(async (id: string, patch: Partial<Task>) => {
|
||||
const current = tasks.get(id);
|
||||
if (!current) throw new Error(`Task ${id} missing`);
|
||||
const next = { ...current, ...patch } as Task;
|
||||
tasks.set(id, next);
|
||||
return next;
|
||||
}),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
parseFileScopeFromPrompt: vi.fn().mockResolvedValue(["packages/engine/src/self-healing.ts"]),
|
||||
getCompletionHandoffAcceptedMarker: vi.fn().mockReturnValue(null),
|
||||
listWorkflowDefinitions: vi.fn().mockResolvedValue([{ ir: RENAMED_BOARD_IR }]),
|
||||
/* A real renamed board has a SELECTION; without it every card resolves to the built-in
|
||||
workflow and `shipped` is not recognised as complete, so the sweep finds nothing to do. */
|
||||
getTaskWorkflowSelection: vi.fn(() => ({ workflowId: RENAMED_BOARD_IR.id, stepIds: [] })),
|
||||
getTaskWorkflowSelectionAsync: vi.fn(async () => ({ workflowId: RENAMED_BOARD_IR.id, stepIds: [] })),
|
||||
getWorkflowDefinition: vi.fn(async () => ({ ir: RENAMED_BOARD_IR })),
|
||||
recordRunAuditEvent: vi.fn().mockResolvedValue(undefined),
|
||||
} as unknown as TaskStore;
|
||||
return { store, tasks };
|
||||
}
|
||||
|
||||
|
||||
it("preserves an overlap blocker resting in a RENAMED wip column", async () => {
|
||||
const staleBlocker = makeTask("FN-DONE-BLOCKER", { column: "shipped" });
|
||||
const overlapBlocker = makeTask("FN-ACTIVE-OVERLAP", { column: "building" });
|
||||
const dependent = makeTask("FN-DEPENDENT", {
|
||||
column: "drafting",
|
||||
status: "queued",
|
||||
blockedBy: staleBlocker.id,
|
||||
overlapBlockedBy: overlapBlocker.id,
|
||||
dependencies: [staleBlocker.id],
|
||||
});
|
||||
const { store, tasks } = createRenamedBoardStore([staleBlocker, overlapBlocker, dependent]);
|
||||
const manager = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getExecutingTaskIds: () => new Set<string>(),
|
||||
});
|
||||
|
||||
await expect(manager.clearStaleBlockedBy()).resolves.toBe(1);
|
||||
|
||||
/* The lease is still held, so the overlap blocker survives the stale-dependency clear. */
|
||||
expect(tasks.get(dependent.id)?.overlapBlockedBy).toBe(overlapBlocker.id);
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
dependent.id,
|
||||
expect.stringContaining(`still blocked by file scope overlap with ${overlapBlocker.id}`),
|
||||
);
|
||||
|
||||
manager.stop();
|
||||
});
|
||||
|
||||
it("preserves an overlap blocker resting in a RENAMED review column", async () => {
|
||||
/* The review half of the predicate, which takes a different branch (worktree + status). */
|
||||
const staleBlocker = makeTask("FN-DONE-BLOCKER", { column: "shipped" });
|
||||
const overlapBlocker = makeTask("FN-ACTIVE-OVERLAP", {
|
||||
column: "checking",
|
||||
worktree: "/tmp/wt-active",
|
||||
});
|
||||
const dependent = makeTask("FN-DEPENDENT", {
|
||||
column: "drafting",
|
||||
status: "queued",
|
||||
blockedBy: staleBlocker.id,
|
||||
overlapBlockedBy: overlapBlocker.id,
|
||||
dependencies: [staleBlocker.id],
|
||||
});
|
||||
const { store, tasks } = createRenamedBoardStore([staleBlocker, overlapBlocker, dependent]);
|
||||
const manager = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getExecutingTaskIds: () => new Set<string>(),
|
||||
});
|
||||
|
||||
await expect(manager.clearStaleBlockedBy()).resolves.toBe(1);
|
||||
|
||||
expect(tasks.get(dependent.id)?.overlapBlockedBy).toBe(overlapBlocker.id);
|
||||
|
||||
manager.stop();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -32,6 +32,8 @@ const FAMILY = [
|
||||
role: "hold" as const,
|
||||
column: "todo",
|
||||
renamedColumn: "drafting",
|
||||
/* The SECOND column carrying this row's role in `irSplitRole`. */
|
||||
splitColumn: "blocked",
|
||||
task: (over: Partial<Task> = {}) => ({ paused: true, pausedReason: "manual-hold", ...over }),
|
||||
},
|
||||
{
|
||||
@@ -41,6 +43,7 @@ const FAMILY = [
|
||||
role: "review" as const,
|
||||
column: "in-review",
|
||||
renamedColumn: "checking",
|
||||
splitColumn: "signoff",
|
||||
task: (over: Partial<Task> = {}) => ({ paused: true, pausedReason: "manual-hold", ...over }),
|
||||
},
|
||||
{
|
||||
@@ -50,6 +53,7 @@ const FAMILY = [
|
||||
role: "review" as const,
|
||||
column: "in-review",
|
||||
renamedColumn: "checking",
|
||||
splitColumn: "signoff",
|
||||
// This one watches ACTIVE review work, so the card must NOT be paused.
|
||||
task: (over: Partial<Task> = {}) => ({ paused: false, ...over }),
|
||||
},
|
||||
@@ -92,6 +96,29 @@ function ir(hold: string, review: string): WorkflowIr {
|
||||
} as unknown as WorkflowIr;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-21:50 (a role is a TRAIT, so several columns may carry it):
|
||||
A board that parks dependency-blocked cards in their own hold lane, and splits human sign-off from
|
||||
the merge lane, carries each role TWICE. `resolveLifecycleColumns()[role]` answers with the first
|
||||
only, which is what made the surfacing family skip every card resting in the second.
|
||||
*/
|
||||
function irSplitRole(): WorkflowIr {
|
||||
return {
|
||||
version: "v2",
|
||||
id: WF,
|
||||
nodes: [],
|
||||
edges: [],
|
||||
columns: [
|
||||
{ id: "todo", name: "todo", traits: [{ trait: "hold", config: { release: "capacity" } }] },
|
||||
{ id: "blocked", name: "blocked", traits: [{ trait: "hold", config: { release: "dependency" } }] },
|
||||
{ id: "building", name: "building", traits: [{ trait: "wip", config: { limitSetting: "maxConcurrent" } }] },
|
||||
{ id: "in-review", name: "in-review", traits: [{ trait: "merge" }] },
|
||||
{ id: "signoff", name: "signoff", traits: [{ trait: "human-review" }] },
|
||||
{ id: "shipped", name: "shipped", traits: [{ trait: "complete" }] },
|
||||
],
|
||||
} as unknown as WorkflowIr;
|
||||
}
|
||||
|
||||
function harness(tasks: Task[], settings: Record<string, unknown>, workflowIr: WorkflowIr) {
|
||||
/* Appends to the card's log, because the at-most-once dedup READS that log —
|
||||
a mock that only records calls cannot observe suppression at all. */
|
||||
@@ -190,6 +217,71 @@ describe("surfacing family — shared invariants (one row per sweep)", () => {
|
||||
expect(await run(h.manager)).toBe(0);
|
||||
});
|
||||
|
||||
it("fires for a card in the SECOND column carrying its role", async () => {
|
||||
/*
|
||||
The defect this replaces: the role gate compared `task.column` against the FIRST column
|
||||
carrying the role, so a card resting in a board's second hold/review lane was skipped
|
||||
entirely — no diagnostic, no error, and every other assertion in this file still green
|
||||
because the single-role-column fixture could not express the case.
|
||||
*/
|
||||
const h = harness(
|
||||
[makeTask(spec.splitColumn, spec.task())],
|
||||
{ [spec.thresholdKey]: CUSTOMIZED_MS },
|
||||
irSplitRole(),
|
||||
);
|
||||
|
||||
expect(await run(h.manager)).toBe(1);
|
||||
expect(h.logEntry).toHaveBeenCalledWith("FN-1", expect.stringContaining(spec.logPrefix));
|
||||
});
|
||||
|
||||
it("still fires for a card in the FIRST role column when the role is split", async () => {
|
||||
/* Membership must WIDEN the gate, not move it. */
|
||||
const h = harness(
|
||||
[makeTask(spec.column, spec.task())],
|
||||
{ [spec.thresholdKey]: CUSTOMIZED_MS },
|
||||
irSplitRole(),
|
||||
);
|
||||
|
||||
expect(await run(h.manager)).toBe(1);
|
||||
});
|
||||
|
||||
it("reads the recovery policy of the card's OWN role column, not the first one", async () => {
|
||||
/*
|
||||
A second first-match bug hides inside the fix for the first: resolving membership but still
|
||||
reading `roleColumns[0]`'s declared policy applies the merge lane's threshold to a card in the
|
||||
sign-off lane. Here the FIRST role column declares a policy that would suppress the signal and
|
||||
the card's own column declares one that fires — so reading the wrong column returns 0.
|
||||
*/
|
||||
const workflow = irSplitRole() as unknown as { columns: Array<{ id: string; recovery?: unknown }> };
|
||||
workflow.columns.find((c) => c.id === spec.column)!.recovery = {
|
||||
stalenessMs: BUILTIN_DEFAULT_MS,
|
||||
onStale: { action: "surface", code: spec.logPrefix },
|
||||
};
|
||||
workflow.columns.find((c) => c.id === spec.splitColumn)!.recovery = {
|
||||
stalenessMs: CUSTOMIZED_MS,
|
||||
onStale: { action: "surface", code: spec.logPrefix },
|
||||
};
|
||||
|
||||
const h = harness(
|
||||
[makeTask(spec.splitColumn, spec.task())],
|
||||
{ [spec.thresholdKey]: BUILTIN_DEFAULT_MS },
|
||||
workflow as unknown as WorkflowIr,
|
||||
);
|
||||
|
||||
expect(await run(h.manager)).toBe(1);
|
||||
});
|
||||
|
||||
it("does NOT fire for a card outside every column carrying its role", async () => {
|
||||
/* Membership must still narrow: `building` carries neither role. */
|
||||
const h = harness(
|
||||
[makeTask("building", spec.task())],
|
||||
{ [spec.thresholdKey]: CUSTOMIZED_MS },
|
||||
irSplitRole(),
|
||||
);
|
||||
|
||||
expect(await run(h.manager)).toBe(0);
|
||||
});
|
||||
|
||||
it("is AT-MOST-ONCE: a card already surfaced inside the window is not re-reported", async () => {
|
||||
/* The safeguard that lives outside the policy table. Without it a stale
|
||||
card is re-reported every poll, which trains operators to ignore the log. */
|
||||
|
||||
@@ -4735,11 +4735,24 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
|
||||
const hasActiveFileScopeOverlapBlocker = async (dependent: Task, blockerId: string | null | undefined): Promise<boolean> => {
|
||||
if (!blockerId) return false;
|
||||
const blocker = taskById.get(blockerId);
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-22:20 (shared predicate, HALF-converted):
|
||||
`shouldHoldActiveFileScopeLease` is the scheduler's lease predicate, shared with this path on
|
||||
purpose so stale-blocker cleanup cannot preserve blockers the scheduler would ignore — or, as
|
||||
here, RELEASE blockers the scheduler still honours. Its two role answers are optional
|
||||
parameters defaulting to the legacy ids; the scheduler's own call sites pass resolved answers
|
||||
and these did not, so on a renamed board the two disagreed: the scheduler kept the lease while
|
||||
this sweep saw `false` for every card, cleared `overlapBlockedBy`, and released a dependent to
|
||||
edit files another agent still holds. Membership comes from the sets this sweep already
|
||||
resolved a few lines above.
|
||||
*/
|
||||
if (!blocker || !shouldHoldActiveFileScopeLease(blocker, allTasks, {
|
||||
mergeRequestContractShadowEnabled: settings.mergeRequestContractShadowEnabled,
|
||||
handoffAccepted: settings.mergeRequestContractShadowEnabled === true
|
||||
? (await this.store.getCompletionHandoffAcceptedMarker(blocker.id)) !== null
|
||||
: false,
|
||||
isWipColumn: completedWipColumns.has(blocker.column),
|
||||
isReviewColumn: completedReviewColumns.has(blocker.column),
|
||||
})) return false;
|
||||
const dependentScope = await getFilteredFileScope(dependent.id);
|
||||
if (dependentScope.length === 0 || isCoordinationOnlyTask(dependent, dependentScope)) return false;
|
||||
@@ -5793,11 +5806,24 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
|
||||
const hasActiveFileScopeOverlapBlocker = async (task: Task, blockerId: string | null | undefined): Promise<boolean> => {
|
||||
if (!blockerId) return false;
|
||||
const blocker = taskById.get(blockerId);
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-22:20 (shared predicate, HALF-converted):
|
||||
`shouldHoldActiveFileScopeLease` is the scheduler's lease predicate, shared with this path on
|
||||
purpose so stale-blocker cleanup cannot preserve blockers the scheduler would ignore — or, as
|
||||
here, RELEASE blockers the scheduler still honours. Its two role answers are optional
|
||||
parameters defaulting to the legacy ids; the scheduler's own call sites pass resolved answers
|
||||
and these did not, so on a renamed board the two disagreed: the scheduler kept the lease while
|
||||
this sweep saw `false` for every card, cleared `overlapBlockedBy`, and released a dependent to
|
||||
edit files another agent still holds. Membership comes from the sets this sweep already
|
||||
resolved a few lines above.
|
||||
*/
|
||||
if (!blocker || !shouldHoldActiveFileScopeLease(blocker, allTasks, {
|
||||
mergeRequestContractShadowEnabled: settings.mergeRequestContractShadowEnabled,
|
||||
handoffAccepted: settings.mergeRequestContractShadowEnabled === true
|
||||
? (await this.store.getCompletionHandoffAcceptedMarker(blocker.id)) !== null
|
||||
: false,
|
||||
isWipColumn: blockedWipColumns.has(blocker.column),
|
||||
isReviewColumn: blockedReviewColumns.has(blocker.column),
|
||||
})) return false;
|
||||
|
||||
const taskScope = await getFilteredFileScope(task.id);
|
||||
@@ -8702,8 +8728,8 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
|
||||
logPrefix: "Stale paused todo surfaced",
|
||||
role: "hold",
|
||||
isEligible: (task) => task.paused === true,
|
||||
evaluate: (task, thresholdMs, now, activation, roleColumn) =>
|
||||
getStalePausedTodoSignal(task, { now, thresholdMs, holdColumn: roleColumn, ...activation }),
|
||||
evaluate: (task, thresholdMs, now, activation, roleColumns) =>
|
||||
getStalePausedTodoSignal(task, { now, thresholdMs, holdColumns: roleColumns, ...activation }),
|
||||
describe: (_task, signal, thresholdMs) =>
|
||||
`paused ${hours(signal.ageMs)}h beyond ${hours(thresholdMs)}h threshold; disposition options — unpause, move to triage, archive, or create follow-up task. pausedReason=${(_task as { pausedReason?: string }).pausedReason ?? "none"}`,
|
||||
},
|
||||
@@ -8719,8 +8745,8 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
|
||||
logPrefix: "Stale paused review surfaced",
|
||||
role: "review",
|
||||
isEligible: (task) => task.paused === true,
|
||||
evaluate: (task, thresholdMs, now, activation, roleColumn) =>
|
||||
getStalePausedReviewSignal(task, { now, thresholdMs, reviewColumn: roleColumn, ...activation }),
|
||||
evaluate: (task, thresholdMs, now, activation, roleColumns) =>
|
||||
getStalePausedReviewSignal(task, { now, thresholdMs, reviewColumns: roleColumns, ...activation }),
|
||||
describe: (_task, signal) =>
|
||||
`paused ${hours(signal.ageMs)}h; disposition options — unpause, retry, archive, or create follow-up task. pausedReason=${(_task as { pausedReason?: string }).pausedReason ?? "none"}`,
|
||||
},
|
||||
@@ -8754,14 +8780,14 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
|
||||
task.id !== activeMergeTaskId &&
|
||||
!executingTaskIds.has(task.id),
|
||||
isEligibleAsync: async (task) => !(await this.isMergeLaneOwned(task.id)),
|
||||
evaluate: (task, thresholdMs, now, activation, roleColumn) => {
|
||||
evaluate: (task, thresholdMs, now, activation, roleColumns) => {
|
||||
const signal = getInReviewStalledSignal(task, {
|
||||
now,
|
||||
thresholdMs,
|
||||
autoMerge: true,
|
||||
activeMergeTaskId,
|
||||
executingTaskIds,
|
||||
reviewColumn: roleColumn,
|
||||
reviewColumns: roleColumns,
|
||||
...activation,
|
||||
});
|
||||
return signal ? { ...signal, code: signal.code, ageMs: signal.quietMs } : undefined;
|
||||
|
||||
@@ -34,7 +34,8 @@ import {
|
||||
type ResolvedRecoveryPolicy,
|
||||
} from "./recovery-reconciler.js";
|
||||
import {
|
||||
resolveLifecycleColumns,
|
||||
columnsWithFlag,
|
||||
resolveReviewColumns,
|
||||
resolveWorkflowIrForTask,
|
||||
type Settings,
|
||||
type Task,
|
||||
@@ -86,10 +87,11 @@ export interface SurfacingSpec {
|
||||
thresholdMs: number,
|
||||
cycleStartMs: number,
|
||||
activation: { engineActiveSinceMs?: number; engineActivationGraceMs?: number },
|
||||
/* The RESOLVED role column. Signals take it so their own column check stays
|
||||
real — passing `task.column` would make that check tautological and quietly
|
||||
delete it. */
|
||||
roleColumn: string,
|
||||
/* The RESOLVED role column SET. Signals take it so their own column check
|
||||
stays real — passing `task.column` would make that check tautological and
|
||||
quietly delete it. A SET rather than one id because a role is a trait many
|
||||
columns may carry; see `resolveRoleColumns`. */
|
||||
roleColumns: ReadonlySet<string>,
|
||||
): SurfacingSignal | undefined;
|
||||
/** The operator-facing sentence following `logPrefix [code]: `. */
|
||||
describe(task: Task, signal: SurfacingSignal, thresholdMs: number): string;
|
||||
@@ -128,21 +130,42 @@ export interface SurfacingRunnerDeps {
|
||||
* Returns `undefined` when neither supplies an actionable threshold — including
|
||||
* when the operator set a non-positive value, which means DISABLED.
|
||||
*/
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-30-21:45 (surfacing family watched only the FIRST role column):
|
||||
A lifecycle role is a TRAIT, and any number of a board's columns may carry it — a workflow with a
|
||||
merge lane beside a human-review lane has two review columns. `resolveLifecycleColumns()[role]`
|
||||
answers with the FIRST one only, so the role gate below (`task.column !== roleColumn`) dropped every
|
||||
card resting in any other column carrying the same role: no stale-paused-review, no in-review-stalled,
|
||||
no stale-paused-todo diagnostic for those cards, silently and with no error.
|
||||
|
||||
Membership, not first-match. `review` is the union of the three review roles (the same answer
|
||||
`resolveReviewColumns` gives every other converted reader) so a board splitting merge orchestration
|
||||
from human review is covered by both.
|
||||
*/
|
||||
function resolveRoleColumns(ir: WorkflowIr, role: SurfacingSpec["role"]): string[] {
|
||||
return role === "review" ? resolveReviewColumns(ir) : columnsWithFlag(ir, "hold");
|
||||
}
|
||||
|
||||
async function resolveTaskThreshold(
|
||||
store: TaskStore,
|
||||
task: Task,
|
||||
spec: SurfacingSpec,
|
||||
inheritedThresholdMs: number,
|
||||
irCache: Map<string, WorkflowIr>,
|
||||
): Promise<{ policy: ResolvedRecoveryPolicy; roleColumn: string } | undefined> {
|
||||
): Promise<{ policy: ResolvedRecoveryPolicy; roleColumns: ReadonlySet<string> } | undefined> {
|
||||
let declared;
|
||||
let roleColumn: string | undefined;
|
||||
let roleColumns: string[] = [];
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(store, task.id, irCache);
|
||||
const lifecycle = resolveLifecycleColumns(ir);
|
||||
roleColumn = lifecycle?.[spec.role];
|
||||
if (roleColumn && ir.version === "v2") {
|
||||
declared = ir.columns.find((c) => c.id === roleColumn)?.recovery;
|
||||
roleColumns = resolveRoleColumns(ir, spec.role);
|
||||
if (roleColumns.length > 0 && ir.version === "v2") {
|
||||
/*
|
||||
The card's OWN column declares its recovery policy when it carries the role. Reading the
|
||||
first role column's policy instead would apply the merge lane's threshold to a card sitting
|
||||
in the human-review lane — a second first-match bug hiding inside the fix for the first.
|
||||
*/
|
||||
const policyColumn = roleColumns.includes(task.column) ? task.column : roleColumns[0];
|
||||
declared = ir.columns.find((c) => c.id === policyColumn)?.recovery;
|
||||
}
|
||||
} catch {
|
||||
// Unresolvable workflow: fall through to the inherited setting.
|
||||
@@ -156,11 +179,14 @@ async function resolveTaskThreshold(
|
||||
/*
|
||||
A workflow that cannot resolve the role (v1 / no column vocabulary) keeps the
|
||||
LEGACY id, so an unresolvable workflow behaves exactly as before this migration
|
||||
rather than losing the sweep entirely. Never `undefined`, so the role gate in
|
||||
the runner is ALWAYS active — leaving it optional made the gate silently absent
|
||||
for exactly the workflows whose role failed to resolve.
|
||||
rather than losing the sweep entirely. Never empty, so the role gate in the
|
||||
runner is ALWAYS active — leaving it optional made the gate silently absent for
|
||||
exactly the workflows whose role failed to resolve.
|
||||
*/
|
||||
return { policy, roleColumn: roleColumn ?? LEGACY_ROLE_COLUMN[spec.role] };
|
||||
return {
|
||||
policy,
|
||||
roleColumns: new Set(roleColumns.length > 0 ? roleColumns : [LEGACY_ROLE_COLUMN[spec.role]]),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -200,12 +226,12 @@ export async function runSurfacingSweep(spec: SurfacingSpec, deps: SurfacingRunn
|
||||
if (!resolved) continue;
|
||||
const thresholdMs = resolved.policy.stalenessMs;
|
||||
|
||||
/* The role gate: only cards resting in THIS task's own role column. */
|
||||
if (task.column !== resolved.roleColumn) continue;
|
||||
/* The role gate: only cards resting in one of THIS task's own role columns. */
|
||||
if (!resolved.roleColumns.has(task.column)) continue;
|
||||
|
||||
if (spec.isEligibleAsync && !(await spec.isEligibleAsync(task))) continue;
|
||||
|
||||
const signal = spec.evaluate(task, thresholdMs, deps.cycleStartMs, deps.activation, resolved.roleColumn);
|
||||
const signal = spec.evaluate(task, thresholdMs, deps.cycleStartMs, deps.activation, resolved.roleColumns);
|
||||
if (!signal) continue;
|
||||
|
||||
/* A row written during this cycle is re-read next pass, so reporting it
|
||||
|
||||
@@ -8,7 +8,6 @@
|
||||
"packages/engine/src/project-engine.ts": 1,
|
||||
"packages/engine/src/runtimes/in-process-runtime.ts": 1,
|
||||
"packages/engine/src/scheduler.ts": 1,
|
||||
"packages/engine/src/self-healing.ts": 4,
|
||||
"packages/dashboard/src/routes/register-task-workflow-routes.ts": 1,
|
||||
"packages/dashboard/app/components/ExecutorStatusBar.tsx": 1,
|
||||
"packages/dashboard/app/components/Lane.tsx": 1,
|
||||
|
||||
@@ -42,6 +42,9 @@ export const LANE_ARGUMENT_NAMES = new Set([
|
||||
"isReviewColumn",
|
||||
"isWipColumn",
|
||||
"holdColumn",
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-07-30-22:00: the MEMBERSHIP form, added with the surfacing
|
||||
family's split-role fix — without it the gate cannot see a dropped `holdColumns`. */
|
||||
"holdColumns",
|
||||
]);
|
||||
|
||||
function parse(file) {
|
||||
|
||||
Reference in New Issue
Block a user