Skip to content

fix(scheduler): one runner for the install, not one per engine - #594

Closed
rainmanjam wants to merge 1 commit into
mainfrom
fix/scheduler-install-wide
Closed

fix(scheduler): one runner for the install, not one per engine#594
rainmanjam wants to merge 1 commit into
mainfrom
fix/scheduler-install-wide

Conversation

@rainmanjam

Copy link
Copy Markdown
Owner

Closes #526.

The bug

schedules has no source_id — a timetable describes the box. But every engine built its own scheduler.Runner over that one table.

Whichever runner swept first wrote enabled on every destination (ListDestinationIDs is install-wide, correctly), then called Reconcile — which was engine-local — and marked the occurrence handled. The other engines read it as handled and never reconciled.

Their destinations sat enabled in the database with no process publishing, and the log said schedule fired.

For a product where a completed broadcast cannot return to live, a scheduled show that silently doesn't start while reporting success is the worst failure available. On a stop it inverts: keeps publishing while the DB says stopped.

MarkScheduleRun's WHERE id=? AND last_run_at < ? is a ratchet on the row, not a lease over the work — and MarkScheduleRun returns only an error, so a runner cannot discover it lost. Safety depended on sweep timing.

The fix

The actuator was already install-wide in everything but Reconcile: both writes go through store, and ListDestinationIDs deliberately spans the install. Moving it to the Manager makes all four consistent and makes the mistake unrepresentable rather than unlikely.

Also closes the handleScheduleRuns half of the default-engine register — the runs page reported programme 1's scheduler as though there were several timetables. There is one.

The test took three attempts and the first two were worthless

  • "A runner exists" — passes with per-engine runners restored.
  • "At least 3 engines" — passes too; managerFixture already creates a default source, so the threshold was met before the reconcile.
  • A delta — a source created after Start has no engine; Manager.Reconcile calls Sync and builds one; an engine cannot create its siblings.

All three were mutation-checked. Only the third failed the mutation, which is the only reason it's trusted.

Three of the repo's own devices refused the change until it was complete: the default-engine register, the nil-receiver classification list, and its assertion that the list matches the real method set.

gofmt, go vet, full suite green.

https://claude.ai/code/session_01A8N3W5ct9SZtHK9sCDD9cL

Closes #526. Also closes the handleScheduleRuns half of the default-engine
register, which reported programme 1's runs as though there were several
timetables.

`schedules` has no source_id: a timetable describes the box. Every engine
nonetheless built its own scheduler.Runner over that one table, and the failure
was structural rather than a race that sometimes lost. Whichever runner swept
first wrote `enabled` on EVERY destination -- ListDestinationIDs is install-wide
and correctly so -- then called Reconcile, which was engine-local, and marked
the occurrence handled. The remaining engines read it as handled and never
reconciled. Their destinations were enabled in the database with nothing
publishing, and the log said `schedule fired`.

For a product where a completed broadcast cannot return to live, a scheduled
show that does not start while reporting success is the worst failure available.
On a `stop` it inverts: keeps publishing while the database says stopped.

MarkScheduleRun's `WHERE id=? AND last_run_at < ?` is a ratchet on the row, not
a lease over the work, and Actuator.MarkScheduleRun returns only an error -- a
runner cannot discover that it lost. So safety depended on sweep timing.

The actuator was already install-wide in everything but Reconcile: both writes
go through store, and ListDestinationIDs deliberately spans the install. Moving
it to the Manager makes all four consistent and makes the mistake unrepresentable
rather than unlikely, which is the difference between a device and a habit.

THE TEST TOOK THREE ATTEMPTS AND THE FIRST TWO WERE WORTHLESS. Asserting that a
runner exists passes with the per-engine runners restored. Asserting an absolute
engine count passes too, because managerFixture already creates a default source
-- the threshold was met before the reconcile ran. What discriminates is a
DELTA: a source created after Start has no engine, Manager.Reconcile calls Sync
and builds one, and an engine cannot create its siblings. Both earlier versions
were mutation-checked and both survived the mutation, which is the only reason
this one is trusted.

Three of the repo's own devices refused the change until it was complete: the
default-engine register (stale handleScheduleRuns entry), the nil-receiver
classification list, and its assertion that the list matches the real method set.

Claude-Session: https://claude.ai/code/session_01A8N3W5ct9SZtHK9sCDD9cL
Copilot AI lite review requested due to automatic review settings August 26, 2026 20:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
71.4% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

scheduler: every engine runs its own scheduler over one install-wide schedules table, and whichever one fires reconciles only itself

2 participants