fleet(engine): self-healing 6 → 1 — the board-stall counter, the last guard that needed a sync answer (#3121)
The last fan-out guard, and the one I explicitly said needed a synchronous answer. #3109 made that answer available without an await, so the flag comes off. ## Why this one was last The other two guards in this listener gated work the listener **already `void`s**, so they moved onto the async resolver in #3094. This one increments in-memory state **in the handler's own tick**, so it genuinely needed a synchronous answer. The sync IR path was never that answer: `resolveTaskWorkflowIrSync` cannot resolve a **custom** workflow at all — two independent blockers, #3103 — which is why I wrote that conversion, measured it, and withdrew it. #3109's emitter-carried `lanes` removes the dilemma rather than trading one horn for the other: reading them needs **no await**, so the increment stays in the same tick *and* the guard becomes correct. ## What it fixes On a renamed board this counter read **zero**. The board-stall watchdog was blind to a board whose cards were moving out of implementation the whole time — the signal it exists to raise was never raised. ## Census | | before | after | |---|---|---| | `self-healing.ts` | 6 | **1** | | repo backlog | 29 | **24** | The remaining 1 is the log-dedup closure — a pre-existing flag whose degraded answer costs a duplicate log line, not a lifecycle decision. ## Measured - 3 new cases; `self-healing-completion-fanout.test.ts` **13/13 pass**. - **MUTATION**: restoring the literal pair fails the renamed case. - **The paired negative is the load-bearing one.** The guard means *"left implementation for somewhere that is not implementation"*, so a move **between two non-wip lanes** must not count. Without that case, a conversion that counted every move would pass the positive and inflate the watchdog's denominator — breaking it in the opposite direction, which is harder to notice than a zero. - A **fail-soft** case pins that an emit carrying no `lanes` still counts on the legacy ids. - **Asserted through the counter itself**, not a downstream alert. The increment *is* what this guard decides; routing the assertion through the watchdog would let an unrelated threshold change mask a regression here. - `src/__tests__/self-healing*` + `task-agent*` — **42 files / 848 tests pass**. - `tsc --noEmit -p packages/engine` clean; census `--strict`, `check-lane-wiring`, `check-inert-sync-lane-conversions`, `check-fnxc-future-dates` clean. ## On the withdrawal this reverses #3094 withdrew a sync-IR conversion of this listener and recorded why, precisely. That record is what made this cheap: I could tell in one read that #3109 addressed the *specific* obstacle rather than a general "async is hard". A flag that names its blocker exactly is a flag that can be retired the day the blocker goes. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -292,3 +292,76 @@ describe("the task:moved fan-out resolves the board's own lanes", () => {
|
||||
mgr.stop();
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-23:45:
|
||||
THE BOARD-STALL COUNTER, the last fan-out guard and the only one that needed a SYNCHRONOUS answer.
|
||||
|
||||
It increments in-memory state in the handler's own tick, so it could not follow the other two guards
|
||||
onto the async resolver, and the sync IR path cannot resolve a custom workflow at all — a conversion
|
||||
through it would have been inert. #3109's emitter-carried `lanes` removes the dilemma: reading them
|
||||
needs no await, so the increment stays in the same tick and the guard becomes correct.
|
||||
|
||||
On a renamed board this counter read ZERO, so the board-stall watchdog was blind to a board whose
|
||||
cards were moving out of implementation the whole time.
|
||||
|
||||
Asserted through the counter itself rather than a downstream alert: the increment IS what the guard
|
||||
decides, and routing the assertion through the watchdog would let an unrelated threshold change mask
|
||||
a regression here.
|
||||
*/
|
||||
describe("the board-stall counter follows the board's own lanes", () => {
|
||||
const RENAMED_LANES = { hold: "drafting", intake: "inbox", wip: "building", review: "checking", complete: "shipped", archived: "filed" };
|
||||
|
||||
function startedManager(store: TaskStore & EventEmitter) {
|
||||
const mgr = new SelfHealingManager(store, { rootDir: "/repo" });
|
||||
vi.spyOn(mgr as unknown as { startMaintenance: () => void }, "startMaintenance").mockImplementation(() => {});
|
||||
vi.spyOn(mgr, "reconcileCompletedTask").mockResolvedValue({ blockedByCleared: 0, worktreeRemoved: false, branchRemoved: false });
|
||||
vi.spyOn(mgr, "reconcileInReviewBranchRebind").mockResolvedValue(0 as never);
|
||||
mgr.start();
|
||||
(mgr as unknown as { boardStallWindow: { transitionsOutOfInProgressInWindow: number } }).boardStallWindow =
|
||||
{ transitionsOutOfInProgressInWindow: 0 };
|
||||
return mgr;
|
||||
}
|
||||
|
||||
const counterOf = (mgr: SelfHealingManager) =>
|
||||
(mgr as unknown as { boardStallWindow: { transitionsOutOfInProgressInWindow: number } }).boardStallWindow
|
||||
.transitionsOutOfInProgressInWindow;
|
||||
|
||||
it("counts a move out of the RENAMED wip lane into the renamed review lane", () => {
|
||||
const t = makeTask("FN-C1", { column: "checking" });
|
||||
const store = createStore([t]);
|
||||
const mgr = startedManager(store);
|
||||
|
||||
store.emit("task:moved", { task: t, from: "building", to: "checking", source: "engine", lanes: RENAMED_LANES });
|
||||
|
||||
expect(counterOf(mgr)).toBe(1);
|
||||
mgr.stop();
|
||||
});
|
||||
|
||||
/*
|
||||
The paired negative. The guard is "left implementation for somewhere that is NOT implementation",
|
||||
so a move BETWEEN two non-wip lanes must not count — otherwise the watchdog's denominator inflates
|
||||
and it stops firing for the opposite reason.
|
||||
*/
|
||||
it("does NOT count a move that did not leave the wip lane", () => {
|
||||
const t = makeTask("FN-C2", { column: "shipped" });
|
||||
const store = createStore([t]);
|
||||
const mgr = startedManager(store);
|
||||
|
||||
store.emit("task:moved", { task: t, from: "checking", to: "shipped", source: "engine", lanes: RENAMED_LANES });
|
||||
|
||||
expect(counterOf(mgr)).toBe(0);
|
||||
mgr.stop();
|
||||
});
|
||||
|
||||
it("falls back to the legacy ids when the emitter sent no lanes", () => {
|
||||
const t = makeTask("FN-C3", { column: "in-review" });
|
||||
const store = createStore([t]);
|
||||
const mgr = startedManager(store);
|
||||
|
||||
store.emit("task:moved", { task: t, from: "in-progress", to: "in-review", source: "engine" });
|
||||
|
||||
expect(counterOf(mgr)).toBe(1);
|
||||
mgr.stop();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -30,7 +30,7 @@ import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync,
|
||||
import { readFile } from "node:fs/promises";
|
||||
import { tmpdir } from "node:os";
|
||||
import { isAbsolute, join, relative, resolve } from "node:path";
|
||||
import { resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, getBuiltinWorkflow, isBuiltinWorkflowId, resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance, resolveReboundTarget, columnsWithFlag, resolveLifecycleColumns, resolveTaskLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult, type WorkflowIr,
|
||||
import { type TaskMoveLanes, resolveColumnFlags, IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, resolveEffectiveAutoMerge, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, evaluateCompletedPromotionFailureProvenance, evaluateSkipBypassTaint, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, isNearDuplicateCanonicalInactive, parseExplicitDuplicateMarker, flagTriageDuplicate, isTriageDuplicateKeepAcknowledged, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, getBuiltinWorkflow, isBuiltinWorkflowId, resolveWorkflowIrForTask, resolveWorkflowIrForTaskWithProvenance, resolveReboundTarget, columnsWithFlag, resolveLifecycleColumns, resolveTaskLifecycleColumns, workflowHasColumn, planLegacyAdoption, resolveOrphanedPendingStepResults, classifyReviewLease, PLAN_REVIEW_LEASE_STALENESS_MS, DEFAULT_MAX_POST_REVIEW_FIXES, ACTIVE_WORKFLOW_WORK_ITEM_STATES, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult, type WorkflowIr,
|
||||
LEGACY_COLUMN_IDS_BY_ROLE,
|
||||
TERMINAL_ROLES,
|
||||
resolveProjectColumnsForRoles,
|
||||
@@ -877,7 +877,9 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
|
||||
|
||||
// ── Event listener cleanup ──────────────────────────────────────────
|
||||
private settingsListener: ((data: { settings: Settings; previous: Settings }) => void) | null = null;
|
||||
private taskMovedFanoutListener: ((data: { task: Task; from: string; to: string; source: string }) => void) | null = null;
|
||||
/* FNXC:WorkflowResolvedColumns 2026-07-31-23:40: `lanes` is the emitter-resolved payload #3109
|
||||
added; optional, because an emit path that cannot resolve sends none. */
|
||||
private taskMovedFanoutListener: ((data: { task: Task; from: string; to: string; source: string; lanes?: TaskMoveLanes }) => void) | null = null;
|
||||
|
||||
// ── Per-task deadlock recovery cooldown ─────────────────────────────
|
||||
private deadlockRecoveryCooldown: Map<string, number> = new Map();
|
||||
@@ -1543,43 +1545,40 @@ export class SelfHealingManager extends SelfHealingGitEvidence {
|
||||
this.store.on("settings:updated", this.settingsListener);
|
||||
|
||||
/*
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-23:50 (FLAGGED AND LEFT COUNTED — a conversion I wrote,
|
||||
measured, and withdrew):
|
||||
FNXC:WorkflowResolvedColumns 2026-07-31-23:40 (the last fan-out guard, converted — #3109):
|
||||
THE BOARD-STALL COUNTER WAS THE ONE GUARD HERE THAT GENUINELY NEEDED A SYNCHRONOUS ANSWER, because
|
||||
it mutates in-memory state in the handler's own tick. The other two gated work the listener already
|
||||
`void`s and were converted through the async resolver; this one could not follow them.
|
||||
|
||||
These four guards are dead on a renamed board: the stall counter reads zero, the review rebind
|
||||
never runs, and the completion fan-out never reclaims a worktree or clears a dependent's
|
||||
`blockedBy`. All real. The obvious conversion does NOT fix them.
|
||||
The sync IR path was never the answer for it either: `resolveTaskWorkflowIrSync` cannot resolve a
|
||||
CUSTOM workflow at all (two independent blockers, `sync-workflow-ir-second-blocker.test.ts`), so a
|
||||
conversion routed through it would have been inert — which is why I wrote that conversion, measured
|
||||
it, and withdrew it.
|
||||
|
||||
`task:moved` is emitted synchronously, so an `await` here defers everything after it to a
|
||||
microtask and reorders this handler against every other subscriber — which points at the store's
|
||||
sync IR path, the way the scheduler's `resolveTaskParkedColumnsSync` does. I built exactly that
|
||||
and it is INERT:
|
||||
#3109 removed the dilemma by having the EMITTER carry resolved lanes on the `task:moved` payload.
|
||||
Reading them needs NO await, so the counter's increment stays in the same tick and the guard becomes
|
||||
correct at the same time.
|
||||
|
||||
`getTaskWorkflowSelectionImpl` returns `undefined` UNCONDITIONALLY — "Backend mode cannot
|
||||
synchronously read PostgreSQL" — so `resolveTaskWorkflowIrSync` always takes its `!workflowId`
|
||||
branch and answers with the DEFAULT builtin IR. `columnsWithFlag` on that IR yields exactly
|
||||
`todo / in-progress / in-review / done / archived`. Identical to the literals, on every board.
|
||||
`lanes` is optional and fail-soft to `undefined` — "unknown", never "legacy" — so the literals stay
|
||||
as the fallback, matching the `mergeParkedColumns` convention in `scheduler.ts`.
|
||||
|
||||
Worse than the literal, because the literal is COUNTED: four guards would leave the census, the
|
||||
file would read as converted, and the next reader would have no reason to look. My own test
|
||||
passed only because its store mock supplied a renamed IR — it pinned the helper's shape, not
|
||||
production behaviour. Same defect as #3051's ten scheduler guards; see #3058 and the call-site
|
||||
allow-list in `sync-workflow-ir-callsite-allowlist.test.ts`.
|
||||
|
||||
SCOPE, after the async conversion below: this note now covers ONLY the board-stall counter, which
|
||||
mutates in-memory state in the handler's own tick and so genuinely needs a synchronous answer. The
|
||||
other two guards gate work the listener already `void`s, so they ask the ASYNC resolver instead —
|
||||
see the note on them. Splitting the four this way is the whole finding: "the listener is sync" was
|
||||
never the real constraint, "this particular guard's ANSWER is consumed synchronously" is.
|
||||
|
||||
THE REAL BLOCKER for the counter is a sync path that can answer for a CUSTOM workflow, and there
|
||||
are two things in the way, not one (`sync-workflow-ir-second-blocker.test.ts`). Until then the
|
||||
literals here stay, counted.
|
||||
WHAT IT FIXES: on a renamed board this counter read ZERO, so the board-stall watchdog was blind to
|
||||
a board whose cards were moving out of implementation the whole time — the signal it exists to
|
||||
raise was never raised.
|
||||
*/
|
||||
this.taskMovedFanoutListener = ({ task, from, to }) => {
|
||||
this.taskMovedFanoutListener = ({ task, from, to, lanes }) => {
|
||||
const wipLane = lanes?.wip ?? "in-progress";
|
||||
/* "Left implementation for somewhere that is not implementation" — every lane a card can land
|
||||
in out of wip, so the counter sees the transition whatever the board calls its columns. */
|
||||
const outOfWipLanes = new Set([
|
||||
lanes?.hold ?? "todo",
|
||||
lanes?.review ?? "in-review",
|
||||
lanes?.complete ?? "done",
|
||||
lanes?.archived ?? "archived",
|
||||
]);
|
||||
if (
|
||||
from === "in-progress"
|
||||
&& (to === "todo" || to === "in-review" || to === "done" || to === "archived")
|
||||
from === wipLane
|
||||
&& outOfWipLanes.has(to)
|
||||
&& this.boardStallWindow
|
||||
) {
|
||||
// In-memory only counter; resets on engine restart.
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
{
|
||||
"generatedFrom": "node scripts/lifecycle-column-census.mjs --strict --update-baseline",
|
||||
"byFile": {
|
||||
"packages/engine/src/self-healing.ts": 6,
|
||||
"packages/engine/src/executor.ts": 4,
|
||||
"packages/engine/src/auto-merge-finalization.ts": 3,
|
||||
"packages/dashboard/app/utils/taskRevert.ts": 2,
|
||||
@@ -16,7 +15,8 @@
|
||||
"packages/core/src/task-store/task-id-integrity.ts": 1,
|
||||
"packages/dashboard/app/components/ResearchTaskActionModal.tsx": 1,
|
||||
"packages/dashboard/app/components/TaskCard.tsx": 1,
|
||||
"packages/engine/src/notification/notification-service.ts": 1
|
||||
"packages/engine/src/notification/notification-service.ts": 1,
|
||||
"packages/engine/src/self-healing.ts": 1
|
||||
},
|
||||
"deliberateByFile": {
|
||||
"packages/core/src/task-store/async-comments-attachments.ts\u0000archived": 5,
|
||||
|
||||
Reference in New Issue
Block a user