Problem
The schedule import is two phases: diff-schedule computes a plan (Analyse), the admin reviews it, then commit-schedule applies it later. The diff is a snapshot — commit_schedule takes no per-edition lock and does no re-validation, so if the underlying data changes between Analyse and Commit (a concurrent edit, or just a slow review) an outdated plan is applied.
Most visible failure mode: setIdsToArchive is built from the orphan list at Analyse time, so a set added/edited after Analyse can be archived based on stale information.
Notes
commit_schedule is already a single atomic transaction, so a plain pg_advisory_xact_lock would only serialize concurrent commits — it would not make a stale snapshot fresh. A real fix needs re-validation / optimistic concurrency, e.g.:
- pass a diff version/hash (or the edition's
updated_at watermark) from Analyse through to Commit and re-check it inside commit_schedule, aborting if the edition changed; or
- recompute the orphan set inside the commit transaction rather than trusting
setIdsToArchive from the payload.
Severity
Low for a single-core-team admin tool with infrequent imports, but worth hardening before the importer sees heavier or concurrent use.
Context
Split out of the PR #31 review (schedule-ingestion). Deferred deliberately — the correct fix is more than a lock.
Problem
The schedule import is two phases:
diff-schedulecomputes a plan (Analyse), the admin reviews it, thencommit-scheduleapplies it later. The diff is a snapshot —commit_scheduletakes no per-edition lock and does no re-validation, so if the underlying data changes between Analyse and Commit (a concurrent edit, or just a slow review) an outdated plan is applied.Most visible failure mode:
setIdsToArchiveis built from the orphan list at Analyse time, so a set added/edited after Analyse can be archived based on stale information.Notes
commit_scheduleis already a single atomic transaction, so a plainpg_advisory_xact_lockwould only serialize concurrent commits — it would not make a stale snapshot fresh. A real fix needs re-validation / optimistic concurrency, e.g.:updated_atwatermark) from Analyse through to Commit and re-check it insidecommit_schedule, aborting if the edition changed; orsetIdsToArchivefrom the payload.Severity
Low for a single-core-team admin tool with infrequent imports, but worth hardening before the importer sees heavier or concurrent use.
Context
Split out of the PR #31 review (schedule-ingestion). Deferred deliberately — the correct fix is more than a lock.