elsa-workflows / elsa-workflows/elsa-foundation

[Tech debt] Elsa.Server (Apps): ExtensionBuilderStorage god class, dead git stack, and semaphore leak

Open
#421 1 comment 0 reactions 0 assignees View on GitHub
needs-triage tech-debt
Dominant language
C#
Stars
5
Forks
1
Avg merge
3h 52m
Merged PRs (30d)
212

Description

**Severity: medium | Ease: medium-hard** — batched findings from `src/Apps/Elsa.Server/**`.

1. **`ExtensionBuilderStorage.cs`** is a 2,211-line god class: nearly every public method repeats `await _gate.WaitAsync(); try { load state; mutate; save state; } finally { _gate.Release(); }`, re-serializing the whole `state.json` on every call, while the class also owns git shell-out, build/promotion bookkeeping, and template rendering.
2. **Two parallel, inconsistent git-invocation stacks in the same class:** Stack A (`RunGitAsync(string, CancellationToken, params string[])`, no timeout, ~12 call sites) vs. Stack B (`RunGitReadLines`/`RunGitRead`, hard-coded 2000ms timeout that silently returns `""` on timeout, only 1 real call site). `GetActiveBranch`, `IsRepositoryDirty`, and `GetRemoteState` in Stack B are dead code — never called anywhere.
3. **`ExtensionBuilderBuildRunner.cs:146-147,303-315`:** `BuildLocks` (a static `Dictionary` keyed by project id) is never cleaned up when a project is deleted (`DeleteProjectAsync` doesn't touch it) — every project ever created, including deleted ones, leaks a `SemaphoreSlim` for the host process lifetime.
4. **`ElsaWorkflowManagementApi.cs`:** the same N+1 "backfill missing navigation property" loop is copy-pasted 3x (`ListDefinitionsAsync:93-109`, `ListActivitiesAsync:313-317`, `ListActivityDescriptorsAsync:398-402`).
5. **Two byte-identical `ActivityDefinitionImport` records** in different assemblies (`Elsa3/Mapping/Models/` and `Elsa3/Activities/Design/Import/Models/`) — neither referenced anywhere, a latent CS0433 ambiguous-type risk if both assemblies are ever referenced together.

**Proposed fix:** Extract a `WithStateAsync` helper to remove (1)'s repetition and split git/build/template concerns into separate collaborators; delete Stack B's dead methods and standardize on one timeout policy for (2); dispose/remove the lock entry on project deletion for (3); add a batch `GetManyAsync` and share one helper for (4); delete the duplicate record in (5).

_Found via automated codebase quality audit._

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with src/Apps/Elsa.Server/**, especially ExtensionBuilderStorage.cs, ExtensionBuilderBuildRunner.cs:146-147 and 303-315, and ElsaWorkflowManagementApi.cs at the listed methods. Trace callers and references before deciding collaborator boundaries, lock cleanup, shared navigation loading, and duplicate-record removal. Done means the proposed concerns are separated or consolidated, dead git methods and duplicate types have no remaining references, and deleted projects no longer retain lock entries.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, git
Domain
backend, build-system, tooling
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.