fix(plannings): stop the index endpoint silently dropping workers

[renemadsen]

The bug

TimePlanningPlanningService.Index() — the endpoint behind the planning dashboard — fans out one async task per assigned site. Every task did this:

var result = new List<TimePlanningPlanningModel>();   // shared by all tasks
var tasks = assignedSites.Select(async dbAssignedSite => {
    ...                                               // several real awaits
    result.Add(siteModel);                            // UNSYNCHRONISED
    return siteModel;
}).ToList();
await Task.WhenAll(tasks).ConfigureAwait(false);      // return value DISCARDED

The Add runs after the awaits, so on arbitrary thread-pool threads, genuinely in parallel. List<T> is not thread-safe — concurrent Add loses elements.

The dashboard could return fewer workers than exist, with no error and nothing in the log. A worker silently missing from the grid is a missed shift, and it presents as a data problem rather than a race.

The fix

Task.WhenAll already returned exactly the data the shared list was collecting, and it was being thrown away. Build the result from it:

var siteModels = await Task.WhenAll(tasks).ConfigureAwait(false);

var result = siteModels
    .OfType<TimePlanningPlanningModel>()   // drops the "SDK site not found" nulls
    .OrderBy(x => Regex.Replace(x.SiteName, @"\d", ""))
    .ThenBy(x => x.SiteName)
    .ToList();

No lock, no ConcurrentBag — not sharing the state is less code than synchronising it. It also makes tie-ordering deterministic: OrderBy is stable, so sites with equal keys now fall back to input order instead of completion order.

.OfType<>() is a real filter here, not a no-op: the lambda's inferred return type is TimePlanningPlanningModel? under this file's #nullable enable.

Also fixed: innerDbContext was created per task and never disposed — one DbContext and pooled connection leaked per site per dashboard load. The two changes are load-bearing together; without the dispose, the new test's fan-out would exhaust the connection pool.

How this connects to the flaky e1m shard

This is the cause. The shard saw 14 of 16 rows, so the positional #cell3_0 selector addressed a different worker: the spec wrote five shifts, reopened what it thought was the same cell, and found it empty — Expected "01:03", Received "".

Confirmed against the failing run's log: the not found in Sites list warning, the only other path to a short row count, never fired.

Test changes

e1m/dashboard-edit-multishift.spec.ts now reads the worker from the first dialog it opens and asserts every later open matches, instead of trusting the row index. It deliberately does not hardcode which worker row 3 is — that depends on the server's collation of the seeded names (under da it is c d; under the host's en-US default it would be ag ah), so pinning a name would couple the spec to a derivation rather than to the property that matters: all five opens must hit the same row.

It also gates on the plannings/index POST that follows every assigned-site save, which it previously raced, and asserts the save reported success: true so a failed save can't surface as an opaque gate timeout.

PlanningServiceMultiShiftTests gains a contract test, not a regression test — stated plainly because it matters: it asserts every assigned site comes back, every time. A lost update can't be forced deterministically, so it repeats the call. It would not reliably fail against the old code, but it never passes wrongly. No workflow change needed; that class is already in shard c.

Known gaps, deliberate

• b1m, c1m and d1m carry the identical #firstColumn3 / #cell3_0 construct with neither the gate nor the identity check. Out of scope by agreement; the race fix removes the cause for all of them.
• The PlanRegistrationHelper holiday-config double-checked lock has no volatile — benign on x86/x64, not guaranteed on ARM. Separate follow-up.

Tests run only in CI, so CI is the first execution. Local verification was a clean dotnet build (0 errors) plus a TypeScript parse of the spec.

🤖 Generated with Claude Code

[Copilot]

🟢 Approval recommended

No unresolved issues were identified that would block approval.

Pull request overview

Fixes the planning dashboard race that dropped workers during concurrent site processing and prevents per-site DbContext leaks.

Changes:

• Builds results from Task.WhenAll and adds deterministic ordering.
• Disposes per-site DbContexts.
• Adds backend contract coverage and strengthens the E2E worker-identity and reload checks.

File summaries

File	Description
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn/Services/TimePlanningPlanningService/TimePlanningPlanningService.cs	Updated as part of this pull request.
eFormAPI/Plugins/TimePlanning.Pn/TimePlanning.Pn.Test/PlanningServiceMultiShiftTests.cs	Updated as part of this pull request.
eform-client/playwright/e2e/plugins/time-planning-pn/e1m/dashboard-edit-multishift.spec.ts	Updated as part of this pull request.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.