From 1fb53f9924689ebb4f76399f367375eeb3d63f89 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 30 Jul 2026 09:10:02 -0700 Subject: [PATCH] =?UTF-8?q?docs(scheduler):=20the=20task:moved=20arms=20ar?= =?UTF-8?q?e=20blocked=20by=20a=20synchronous=20prologue=20=E2=80=94=20mea?= =?UTF-8?q?sured,=20and=20deliberately=20left=20counted=20(#2771)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## No behaviour change, and deliberately **no markers** The ten `from`/`to` comparisons in `scheduler.ts`'s `task:moved` handler stay **counted** in the census. They are genuinely wrong on a renamed board — real backlog. Marking them `DELIBERATE-LITERAL` would claim *"reviewed, correct"* when the truth is *"reviewed, still broken, blocked on an ordering question"*, and that is the opposite of what #2767's markers were for. This records the blockage instead. I have deferred these twice citing risk; this is the analysis that deferral was standing in for. ## The measured constraint **The handler is `async`, but its prologue is not.** There is no `await` anywhere between the handler's first line and the terminal-blocker branch ~55 lines down. The snapshot invalidation, the PR-monitor start/stop pair, the mission hand-off and the failed-task tracking all run in the **same tick as the emitter**. So hoisting a resolution to convert those arms does not cost "one await" — it converts the **whole prologue into a microtask**, reordering this listener against every other synchronous `task:moved` subscriber and against the emitter's own continuation. That makes `resolveTaskParkedColumnsSync`'s *"SYNCHRONOUS on purpose"* note **load-bearing rather than stale** — verified by measurement, not assumed. I had been treating it as possibly-stale boilerplate. ## Why the two obvious workarounds don't apply - **Resolve lazily inside the branch.** Doesn't help: the *condition* is what needs the lanes, and it is evaluated in the prologue. - **A cheap sync superset prefilter** — the shape that worked in `usage-limit-detector` — needs a literal predicate that cannot wrongly *exclude* on an unknown vocabulary. For *"is `to` the terminal lane?"* no such predicate exists: a renamed board's terminal id is unknown by construction. (That is precisely why the prefilter *was* safe there — literals can only fail to exclude, never over-exclude.) ## What would actually unblock it 1. **Audit the ordering**, then hoist one await and convert all ten together. That is an audit across every `task:moved` emitter and subscriber — not a scheduler-local change, and not something to do speculatively. 2. **Carry the resolved lanes on the event payload**, so no listener resolves at all. This is the only option that scales to the *other* synchronous listeners with the same problem, and it removes the class rather than one instance. I'd recommend (2) if this is worth funding — it is the same shape as the fix that removed the sync-resolution class in #2759, one layer up. ## Verification 11 scheduler suites — **130 passed** · `pnpm test:gate` **158 / 487 / 10 / 71** · `pnpm lint` clean · engine `tsc --noEmit` **0 errors** · `--strict` exits 0, census unchanged at 12 for this file (which is the point). ## Summary by CodeRabbit * **Documentation** * Added internal documentation explaining synchronous event-ordering requirements when resolving task lanes. * Clarified why asynchronous resolution must not be introduced in this scheduling flow. --------- Co-authored-by: Claude Opus 5 (1M context) --- packages/engine/src/scheduler.ts | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/engine/src/scheduler.ts b/packages/engine/src/scheduler.ts index 9d0db2dfea..ae152b0623 100644 --- a/packages/engine/src/scheduler.ts +++ b/packages/engine/src/scheduler.ts @@ -904,6 +904,26 @@ export class Scheduler { * Also handles mission auto-advance: when a linked task completes, * update feature status and potentially activate next pending slice. */ + /* + FNXC:WorkflowLifecycleColumns 2026-07-30-20:10 (fleet — why the arms below are still literals): + + The ten `from`/`to` comparisons here are real backlog, deliberately left counted. The blocker is + ordering, not effort: this handler is `async` but its PROLOGUE is not — there is no `await` + between this line and the terminal-blocker branch (~55 lines down), so the snapshot invalidation, + PR-monitor start/stop, mission hand-off and failed-task tracking all run in the SAME TICK as the + emitter. Hoisting a resolution to convert those arms turns the prologue into a microtask and + reorders this listener against every other synchronous `task:moved` subscriber. That is the + hazard `resolveTaskParkedColumnsSync` exists to avoid — verified, not assumed. + + Lazy resolution inside a branch does not help (the CONDITION needs the lanes). A sync superset + prefilter does not either: it needs a predicate that cannot wrongly EXCLUDE on an unknown + vocabulary, and a renamed board's terminal id is unknown by construction. + + Unblocking it means either auditing every `task:moved` emitter/subscriber for prologue-ordering + dependence and then converting all ten together, or — preferred, since it removes the class + rather than one instance — having the emitter carry the resolved lanes on the event payload so + no listener resolves at all. + */ this.store.on("task:moved", async ({ task, from, to, source }) => { this.lastAutoClaimFingerprint.set(task.id, computeAutoClaimFingerprint(task)); const parked = resolveTaskParkedColumnsSync(this.store, task.id);