26628475560

Committed 29 May 2026 09:05AM UTC coverage: 83.45% (+0.2%) from 83.252%

Build # 26628475560

Build Type

push

github

Committed by

web-flow

Commit Message

refactor: cut code smells and duplication across packages (#373)

* refactor(config): reduce option parsing duplication

* refactor(phase): share executor naming and error wrapping

* refactor(progress): share timestamped write logic

* refactor(web): share progress event construction

* refactor(cmd): consolidate codex model resolution

* refactor(processor): clean internal policy and plan helpers

* docs(plans): add completed refactor plan

* refactor(plan): restore early-exit in FileHasUncompletedCheckbox

reuse Checkbox.IsActionable inline and return on the first uncompleted
actionable checkbox instead of collecting every checkbox before checking.
keeps the dedup goal, restores the original short-circuit scan. addresses
PR #373 review feedback.

Coverage Stats

199 of 203 new or added lines in 17 files covered. (98.03%)

2 existing lines in 2 files now uncovered.

7614 of 9124 relevant lines covered (83.45%)

225.89 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

84.48

/pkg/processor/phase/task.go

package phase

import (
        "context"
        "errors"
        "fmt"
        "io/fs"
        "time"

        "github.com/umputun/ralphex/pkg/plan"
        "github.com/umputun/ralphex/pkg/status"
)

// TaskPhase executes plan tasks until completion.
type TaskPhase struct {
        cfg            Config
        log            TaskLogger
        exec           Executor
        policy         Policy
        prompts        TaskPrompts
        locator        Locator
        deps           *Deps
        breaks         *BreakController
        iterationDelay time.Duration
        retryCount     int
}

// TaskPhaseOpts contains dependencies for TaskPhase.
type TaskPhaseOpts struct {
        Cfg            Config
        Log            TaskLogger
        Exec           Executor
        Policy         Policy
        Prompts        TaskPrompts
        Locator        Locator
        Deps           *Deps
        Breaks         *BreakController
        IterationDelay time.Duration
        RetryCount     int
}

// NewTaskPhase creates a task phase engine.
func NewTaskPhase(opts TaskPhaseOpts) *TaskPhase {
        breaks := opts.Breaks
        if breaks == nil {
                breaks = NewBreakController(opts.Deps)
        }
        return &TaskPhase{
                cfg: opts.Cfg, log: opts.Log, exec: opts.Exec, policy: opts.Policy,
                prompts: opts.Prompts, locator: opts.Locator, deps: opts.Deps, breaks: breaks,
                iterationDelay: opts.IterationDelay, retryCount: opts.RetryCount,
        }
}

// Run executes one plan task per iteration until all actionable task checkboxes are complete.
func (p *TaskPhase) Run(ctx context.Context) error {
        prompt := p.prompts.TaskPrompt()
        retryCount := 0

        for i := 1; i <= p.cfg.MaxIterations; i++ {
                select {
                case <-ctx.Done():
                        return fmt.Errorf("task phase: %w", ctx.Err())
                default:
                }

                taskNum := i
                if pos := p.NextPlanTaskPosition(); pos > 0 {
                        taskNum = pos
                }
                p.log.PrintSection(status.NewTaskIterationSection(taskNum))

                loopCtx, loopCancel := p.breaks.context(ctx)

                execName := p.cfg.executorName()
                execResult := p.policy.Run(loopCtx, p.exec.Run, prompt, execName)
                result := execResult.Result

                manualBreak := p.breaks.isBreak(loopCtx, ctx)
                loopCancel()

                if manualBreak {
                        p.log.Print("session interrupted by break signal")
                        p.breaks.drain()
                        if p.deps.PauseHandler == nil || !p.deps.PauseHandler(ctx) {
                                return ErrUserAborted
                        }
                        p.breaks.drain()
                        i--
                        retryCount = 0
                        continue
                }

                if err := wrapExecutorError(p.policy, result.Error, execName); err != nil {
                        return err
                }

                if execResult.TimedOut {
                        p.log.Print("%s session timed out, retrying task iteration...", execName)
                        continue
                }

                if result.Signal == SignalCompleted {
                        if p.HasUncompletedTasks() {
                                p.log.Print("warning: completion signal received but plan still has [ ] items, continuing...")
                                continue
                        }
                        p.log.PrintRaw("\nall tasks completed, starting code review...\n")
                        return nil
                }

                if result.Signal == SignalFailed {
                        if retryCount < p.retryCount {
                                p.log.Print("task failed, retrying...")
                                retryCount++
                                if err := p.policy.Sleep(ctx, p.iterationDelay); err != nil {
                                        return fmt.Errorf("interrupted: %w", err)
                                }
                                continue
                        }
                        return errors.New("task execution failed after retry (FAILED signal received)")
                }

                retryCount = 0
                if err := p.policy.Sleep(ctx, p.iterationDelay); err != nil {
                        return fmt.Errorf("interrupted: %w", err)
                }
        }

        return fmt.Errorf("max iterations (%d) reached without completion", p.cfg.MaxIterations)
}

// ValidatePlanHasTasks rejects plan files without executable task sections.
func (p *TaskPhase) ValidatePlanHasTasks() error {
        path := p.locator.Path()
        parsed, err := plan.ParsePlanFile(path)
        if err != nil {
                return fmt.Errorf("parse plan for validation: %w", err)
        }
        if len(parsed.Tasks) == 0 {
                return fmt.Errorf("plan file %q has no executable task sections (### Task N: or ### Iteration N:); add task sections or pass a different plan file", path)
        }
        return nil
}

// HasUncompletedTasks reports whether the current plan still has actionable unchecked task work.
func (p *TaskPhase) HasUncompletedTasks() bool {
        path := p.locator.Path()
        if path == "" {
                return false
        }
        parsed, err := plan.ParsePlanFile(path)
        if err != nil {
                if errors.Is(err, fs.ErrNotExist) {
                        return false
                }
                p.log.Print("[WARN] failed to parse plan file for completion check: %v", err)
                return true
        }
        for _, t := range parsed.Tasks {
                if t.HasUncompletedActionableWork() {
                        return true
                }
        }
        if len(parsed.Tasks) == 0 {
                has, err := plan.FileHasUncompletedCheckbox(path)
                if err != nil {
                        return true
                }
                if has {
                        return true
                }
        }
        return false
}

// NextPlanTaskPosition returns the 1-indexed first uncompleted task position, or zero when unavailable.
func (p *TaskPhase) NextPlanTaskPosition() int {
        parsed, err := plan.ParsePlanFile(p.locator.Path())
        if err != nil {
                p.log.Print("[WARN] failed to parse plan file for task position: %v", err)
                return 0
        }
        for i, t := range parsed.Tasks {
                if t.HasUncompletedActionableWork() {
                        return i + 1
                }
        }
        return 0
}

1	package phase
2
3	import (
4	"context"
5	"errors"
6	"fmt"
7	"io/fs"
8	"time"
9
10	"github.com/umputun/ralphex/pkg/plan"
11	"github.com/umputun/ralphex/pkg/status"
12	)
13
14	// TaskPhase executes plan tasks until completion.
15	type TaskPhase struct {
16	cfg Config
17	log TaskLogger
18	exec Executor
19	policy Policy
20	prompts TaskPrompts
21	locator Locator
22	deps *Deps
23	breaks *BreakController
24	iterationDelay time.Duration
25	retryCount int
26	}
27
28	// TaskPhaseOpts contains dependencies for TaskPhase.
29	type TaskPhaseOpts struct {
30	Cfg Config
31	Log TaskLogger
32	Exec Executor
33	Policy Policy
34	Prompts TaskPrompts
35	Locator Locator
36	Deps *Deps
37	Breaks *BreakController
38	IterationDelay time.Duration
39	RetryCount int
40	}
41
42	// NewTaskPhase creates a task phase engine.
43	func NewTaskPhase(opts TaskPhaseOpts) *TaskPhase {	76✔
44	breaks := opts.Breaks	76✔
45	if breaks == nil {	76✔
46	breaks = NewBreakController(opts.Deps)	×
47	}	×
48	return &TaskPhase{	76✔
49	cfg: opts.Cfg, log: opts.Log, exec: opts.Exec, policy: opts.Policy,	76✔
50	prompts: opts.Prompts, locator: opts.Locator, deps: opts.Deps, breaks: breaks,	76✔
51	iterationDelay: opts.IterationDelay, retryCount: opts.RetryCount,	76✔
52	}	76✔
53	}
54
55	// Run executes one plan task per iteration until all actionable task checkboxes are complete.
56	func (p *TaskPhase) Run(ctx context.Context) error {	9✔
57	prompt := p.prompts.TaskPrompt()	9✔
58	retryCount := 0	9✔
59		9✔
60	for i := 1; i <= p.cfg.MaxIterations; i++ {	26✔
61	select {	17✔
62	case <-ctx.Done():	1✔
63	return fmt.Errorf("task phase: %w", ctx.Err())	1✔
64	default:	16✔
65	}
66
67	taskNum := i	16✔
68	if pos := p.NextPlanTaskPosition(); pos > 0 {	26✔
69	taskNum = pos	10✔
70	}	10✔
71	p.log.PrintSection(status.NewTaskIterationSection(taskNum))	16✔
72		16✔
73	loopCtx, loopCancel := p.breaks.context(ctx)	16✔
74		16✔
75	execName := p.cfg.executorName()	16✔
76	execResult := p.policy.Run(loopCtx, p.exec.Run, prompt, execName)	16✔
77	result := execResult.Result	16✔
78		16✔
79	manualBreak := p.breaks.isBreak(loopCtx, ctx)	16✔
80	loopCancel()	16✔
81		16✔
82	if manualBreak {	19✔
83	p.log.Print("session interrupted by break signal")	3✔
84	p.breaks.drain()	3✔
85	if p.deps.PauseHandler == nil \|\| !p.deps.PauseHandler(ctx) {	4✔
86	return ErrUserAborted	1✔
87	}	1✔
88	p.breaks.drain()	2✔
89	i--	2✔
90	retryCount = 0	2✔
91	continue	2✔
92	}
93
94	if err := wrapExecutorError(p.policy, result.Error, execName); err != nil {	13✔
NEW 95	return err	×
UNCOV 96	}	×
97
98	if execResult.TimedOut {	14✔
99	p.log.Print("%s session timed out, retrying task iteration...", execName)	1✔
100	continue	1✔
101	}
102
103	if result.Signal == SignalCompleted {	16✔
104	if p.HasUncompletedTasks() {	4✔
105	p.log.Print("warning: completion signal received but plan still has [ ] items, continuing...")	×
106	continue	×
107	}
108	p.log.PrintRaw("\nall tasks completed, starting code review...\n")	4✔
109	return nil	4✔
110	}
111
112	if result.Signal == SignalFailed {	13✔
113	if retryCount < p.retryCount {	8✔
114	p.log.Print("task failed, retrying...")	3✔
115	retryCount++	3✔
116	if err := p.policy.Sleep(ctx, p.iterationDelay); err != nil {	3✔
117	return fmt.Errorf("interrupted: %w", err)	×
118	}	×
119	continue	3✔
120	}
121	return errors.New("task execution failed after retry (FAILED signal received)")	2✔
122	}
123
124	retryCount = 0	3✔
125	if err := p.policy.Sleep(ctx, p.iterationDelay); err != nil {	3✔
126	return fmt.Errorf("interrupted: %w", err)	×
127	}	×
128	}
129
130	return fmt.Errorf("max iterations (%d) reached without completion", p.cfg.MaxIterations)	1✔
131	}
132
133	// ValidatePlanHasTasks rejects plan files without executable task sections.
134	func (p *TaskPhase) ValidatePlanHasTasks() error {	4✔
135	path := p.locator.Path()	4✔
136	parsed, err := plan.ParsePlanFile(path)	4✔
137	if err != nil {	4✔
138	return fmt.Errorf("parse plan for validation: %w", err)	×
139	}	×
140	if len(parsed.Tasks) == 0 {	6✔
141	return fmt.Errorf("plan file %q has no executable task sections (### Task N: or ### Iteration N:); add task sections or pass a different plan file", path)	2✔
142	}	2✔
143	return nil	2✔
144	}
145
146	// HasUncompletedTasks reports whether the current plan still has actionable unchecked task work.
147	func (p *TaskPhase) HasUncompletedTasks() bool {	11✔
148	path := p.locator.Path()	11✔
149	if path == "" {	11✔
150	return false	×
151	}	×
152	parsed, err := plan.ParsePlanFile(path)	11✔
153	if err != nil {	12✔
154	if errors.Is(err, fs.ErrNotExist) {	2✔
155	return false	1✔
156	}	1✔
157	p.log.Print("[WARN] failed to parse plan file for completion check: %v", err)	×
158	return true	×
159	}
160	for _, t := range parsed.Tasks {	22✔
161	if t.HasUncompletedActionableWork() {	15✔
162	return true	3✔
163	}	3✔
164	}
165	if len(parsed.Tasks) == 0 {	8✔
166	has, err := plan.FileHasUncompletedCheckbox(path)	1✔
167	if err != nil {	1✔
168	return true	×
169	}	×
170	if has {	2✔
171	return true	1✔
172	}	1✔
173	}
174	return false	6✔
175	}
176
177	// NextPlanTaskPosition returns the 1-indexed first uncompleted task position, or zero when unavailable.
178	func (p *TaskPhase) NextPlanTaskPosition() int {	23✔
179	parsed, err := plan.ParsePlanFile(p.locator.Path())	23✔
180	if err != nil {	25✔
181	p.log.Print("[WARN] failed to parse plan file for task position: %v", err)	2✔
182	return 0	2✔
183	}	2✔
184	for i, t := range parsed.Tasks {	44✔
185	if t.HasUncompletedActionableWork() {	35✔
186	return i + 1	12✔
187	}	12✔
188	}
189	return 0	9✔
190	}

umputun / ralphex / 26628475560

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous