docs(scheduler): the task:moved arms are blocked by a synchronous prologue — measured, and deliberately left counted (#2771)
## 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). <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- 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:
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user