docs(engine): the scheduler flag's reason went stale, and two deferrals read as unexamined (#3142)

My own flag on these two literals went stale, in exactly the way I have
spent this session cataloguing in other people's notes.

## What the note said, and why it is now wrong

It said these two stay because converting them would be **inert** — the
sync resolver answers with the default board. True when written.

#3128 then converted the rest of this listener by deferring each resolve
into a `void (async () => ...)` block, which reaches the **async**
resolver and is genuinely correct. So async resolution *is* available
here now, and my stated reason no longer explains why these two are
different.

## The real reason, which #3128 itself states

Three branches down, in its own note:

> The `planningTaskIds.delete` stays SYNCHRONOUS — it is the
edge-trigger bookkeeping, and deferring it would let a second update
re-enter this branch.

Both remaining literals are that case:

| literal | why it cannot move behind an await |
|---|---|
| `failedTaskIds.add` | edge-trigger bookkeeping raced against
`moveTask` clearing the failure metadata — its own comment says so.
Deferring the add can miss that window. |
| PR-monitoring guard | it gates `getTrackedPrs()` /
`startMonitoring()`, where `tracked.has(task.id)` **is** the re-entrance
guard. Move the lane answer behind an await and two updates for the same
task can both pass that check before either starts — **double-starting a
monitor**. |

## Why the distinction is worth a PR

"Blocked on a resolver" invites the next person to wait for the sync
reader. What these actually need is somewhere to put the answer that is
**not behind an await** — the emitter-carried `lanes` #3109 added to
`task:moved`, whose extension to `task:updated` is measured as expensive
rather than impossible (#3123: 26 emit sites against 7, on the hottest
write path).

Those are different tickets with different owners. Leaving the wrong one
written down is how a blocker outlives its cause — the failure I have
now found in five separate notes this session, including two of my own.

## Measured

- Comment-only.
- `src/__tests__/scheduler*` — **14 files / 144 tests pass**.
- `tsc --noEmit -p packages/engine` clean; `check-inert-sync-lanes` and
census `--strict` clean.
- `check-fnxc-future-dates` is red from `main`'s own #3128 stamps —
**#3139** fixes that; this branch inherits it and does not add to it.

## Census

No movement. Both literals stay counted, now with the correct reason
attached.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
gsxdsm
2026-07-31 10:18:30 -07:00
committed by GitHub
parent a319e35a67
commit 97b945f980
2 changed files with 31 additions and 5 deletions

View File

@@ -1230,8 +1230,31 @@ export class Scheduler {
it. Both live in a synchronous `task:updated` listener, so the async resolver is unavailable
without reordering this handler against every other subscriber.
Unblocking needs a sync-capable workflow-selection reader — one change that un-inerts every
sync-path conversion in this file at once.
FNXC:WorkflowResolvedColumns 2026-07-31-23:59 (THE REASON CHANGED — #3128 made async resolution
available here, so "it would be inert" is no longer why these two stay):
#3128 converted the rest of this listener by deferring the resolve into `void (async () => ...)`
blocks, which reach the ASYNC resolver and are genuinely correct. So the sync-resolver argument
above no longer explains these two. The real reason is the one #3128's own note states, three
branches down:
"The `planningTaskIds.delete` stays SYNCHRONOUS — it is the edge-trigger bookkeeping, and
deferring it would let a second update re-enter this branch."
Both remaining literals are that case:
- `failedTaskIds.add` below is edge-trigger bookkeeping raced against `moveTask` clearing the
failure metadata — the comment on it says so. Deferring the add can miss that window.
- the PR-monitoring guard further down gates `prMonitor.getTrackedPrs()` /
`startMonitoring()`, where `tracked.has(task.id)` IS the re-entrance guard. Move the lane
answer behind an await and two updates for the same task can both pass that check before
either starts, double-starting a monitor.
LEFT COUNTED, both of them: an unconverted literal is visible to the census, and marking these
exempt would assert the code is fine when it is blocked.
So these are not waiting on a resolver. They are waiting on somewhere to put the answer that is
not behind an await — the emitter-carried `lanes` that #3109 added to `task:moved` would do it,
and extending that to `task:updated` is measured as expensive rather than impossible
(`sync-workflow-ir-second-blocker.test.ts`: 26 emit sites against 7, on the hottest write path).
*/
// Track mission failure signals before moveTask clears failure metadata.
if (task.sliceId && task.status === "failed") {
@@ -1318,8 +1341,11 @@ export class Scheduler {
}
if (!this.options.prMonitor) return;
/* FNXC:WorkflowResolvedColumns 2026-07-31-23:58: the second of the two honest literals — see
the note on the mission-failure guard above for why converting it here would be inert. */
/* FNXC:WorkflowResolvedColumns 2026-07-31-23:59: the second of the two honest literals. NOT
because a conversion would be inert — #3128 made the async resolver reachable in this
listener — but because `tracked.has(task.id)` below is a re-entrance guard, and moving this
answer behind an await lets two updates for the same task both pass it and double-start a
monitor. LEFT COUNTED. See the fuller note on the mission-failure guard above. */
if (task.column !== "in-review") return;
if (!task.prInfo) return;

View File

@@ -806,7 +806,7 @@ export class TriageProcessor {
file's census entry pointing at work that is still outstanding.
THE SPECIFICATION IS ABOVE. Whoever supplies a lane answer that is not sync-resolved should make
this line read `disposeLanes.wip` and delete this note.
this line read `disposeLanes.wip` and delete this note. LEFT COUNTED until then.
*/
if (task.column === disposeLanes.hold || task.column === disposeLanes.intake || task.column === "in-progress") return;
if (this.activeSubagentSessions.has(task.id)) {