refactor: split internal/workflows/service.go (2673 lines) #449

New issue

Open

opened 2026-07-25 23:31:28 +02:00 by dries · 0 comments

dries commented

2026-07-25 23:31:28 +02:00

Owner

Code-quality audit (P2). internal/workflows/service.go is 2673 lines and holds the wire types, a 50-method Store interface, publishing, triggers, the scheduler, agent driving, budget accounting, and all validation in one file.

Why it matters

Every workflow change touches the same file, so concurrent work conflicts constantly. Reviewers cannot see which layer a diff belongs to. It is also the umbrella for #446 (dispatchLocked), #447 (Store interface), and #448 (validateDefinition) — do those first, then this becomes mostly file moves.

Suggested split

Along seams that already exist — no behaviour change, pure file moves:

types.go — wire types (Definition, Node, Run, Attempt, ...).
store.go — the Store interface (see the split ticket).
triggers.go — already exists; move shouldFire / nextCheck / fire* / drainQueued into it.
dispatch.go — dispatchLocked and the agent-driving helpers.
validate.go — validateDefinition and the validate* siblings.

service.go keeps the Service struct, construction, and the public API surface.

Acceptance criteria

No file in internal/workflows exceeds ~800 lines.
Pure moves: go test ./internal/workflows/... passes unchanged, no signature changes.
Trigger logic lives in the existing triggers.go.

Effort: L.

Blocked by

Best done after #446, #447, #448.

Code-quality audit (P2). `internal/workflows/service.go` is 2673 lines and holds the wire types, a 50-method `Store` interface, publishing, triggers, the scheduler, agent driving, budget accounting, and all validation in one file. ## Why it matters Every workflow change touches the same file, so concurrent work conflicts constantly. Reviewers cannot see which layer a diff belongs to. It is also the umbrella for #446 (`dispatchLocked`), #447 (`Store` interface), and #448 (`validateDefinition`) — do those first, then this becomes mostly file moves. ## Suggested split Along seams that already exist — no behaviour change, pure file moves: - `types.go` — wire types (`Definition`, `Node`, `Run`, `Attempt`, ...). - `store.go` — the `Store` interface (see the split ticket). - `triggers.go` — **already exists**; move `shouldFire` / `nextCheck` / `fire*` / `drainQueued` into it. - `dispatch.go` — `dispatchLocked` and the agent-driving helpers. - `validate.go` — `validateDefinition` and the `validate*` siblings. `service.go` keeps the `Service` struct, construction, and the public API surface. ## Acceptance criteria - [ ] No file in `internal/workflows` exceeds ~800 lines. - [ ] Pure moves: `go test ./internal/workflows/...` passes unchanged, no signature changes. - [ ] Trigger logic lives in the existing `triggers.go`. Effort: L. ## Blocked by Best done after #446, #447, #448.