refactor: split backgroundAutoApprove and lift its coverage #450

New issue

Open

opened 2026-07-25 23:32:21 +02:00 by dries · 0 comments

dries commented

2026-07-25 23:32:21 +02:00

Owner

Code-quality audit (P2). internal/autoapprove/background.go:106-354 — backgroundAutoApprove is ~249 lines and sits at 24.3% line coverage.

func (s *Service) backgroundAutoApprove(
	ctx context.Context,
	platformID platforms.ID,
	adapter platforms.Platform,
	sessionID string,
	permissionID string,
	permission string,
	patterns []string,
	metadata map[string]any,
) {

One body does: enabled-check, safe-command cache short-circuit, directory resolution, delay read, model mutation, delay sleep, prompt-section loading, recent-message context building, judge call, cancellation re-check, verdict recording, flagged broadcast, cache write, approval persistence.

Why it matters

This is the function that auto-answers permission prompts on the user's behalf — the highest-consequence path in the package. The low coverage is a direct consequence of the size: every test has to set up the whole pipeline to exercise any one branch, so most branches are untested. A regression in the cancellation re-check or the verdict-recording order would auto-approve something it shouldn't and no test would catch it.

Suggested fix

Extract two seams so each is independently testable:

resolveJudgeInputs(...) — directory resolution, delay, model, prompt sections, recent-message context. Returns a struct.
handleUnsafeVerdict(...) — flagged broadcast + the non-approval path.

The remaining body becomes: short-circuit -> resolve inputs -> sleep -> judge -> record -> approve/flag.

Acceptance criteria

backgroundAutoApprove under ~80 lines.
Direct unit tests for both extracted functions.
internal/autoapprove line coverage increases; backgroundAutoApprove and its extractions are collectively above 60%.

Effort: M.

Code-quality audit (P2). `internal/autoapprove/background.go:106-354` — `backgroundAutoApprove` is ~249 lines and sits at **24.3% line coverage**. ```go func (s *Service) backgroundAutoApprove( ctx context.Context, platformID platforms.ID, adapter platforms.Platform, sessionID string, permissionID string, permission string, patterns []string, metadata map[string]any, ) { ``` One body does: enabled-check, safe-command cache short-circuit, directory resolution, delay read, model mutation, delay sleep, prompt-section loading, recent-message context building, judge call, cancellation re-check, verdict recording, flagged broadcast, cache write, approval persistence. ## Why it matters This is the function that auto-answers permission prompts on the user's behalf — the highest-consequence path in the package. The low coverage is a direct consequence of the size: every test has to set up the whole pipeline to exercise any one branch, so most branches are untested. A regression in the cancellation re-check or the verdict-recording order would auto-approve something it shouldn't and no test would catch it. ## Suggested fix Extract two seams so each is independently testable: - `resolveJudgeInputs(...)` — directory resolution, delay, model, prompt sections, recent-message context. Returns a struct. - `handleUnsafeVerdict(...)` — flagged broadcast + the non-approval path. The remaining body becomes: short-circuit -> resolve inputs -> sleep -> judge -> record -> approve/flag. ## Acceptance criteria - [ ] `backgroundAutoApprove` under ~80 lines. - [ ] Direct unit tests for both extracted functions. - [ ] `internal/autoapprove` line coverage increases; `backgroundAutoApprove` and its extractions are collectively above 60%. Effort: M.