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

94.03

/pkg/web/session_progress.go

package web

import (
        "bufio"
        "errors"
        "fmt"
        "io"
        "log"
        "os"
        "strings"
        "time"

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

// ParseProgressHeader reads the header section of a progress file and extracts metadata.
// the header format is:
//
//        # Ralphex Progress Log
//        Plan: path/to/plan.md
//        Branch: feature-branch
//        Mode: full
//        Started: 2026-01-22 10:30:00
//        ------------------------------------------------------------
//
// the second return value reports whether the terminating separator line was
// observed, which means the header is fully written. during a truncate+rewrite
// the header is emitted across several writes, so a mid-write read can return
// incomplete metadata (zero StartTime, missing fields); callers should use the
// complete flag to decide whether to trust the parsed metadata.
func ParseProgressHeader(path string) (meta SessionMetadata, complete bool, err error) {
        f, err := os.Open(path) //nolint:gosec // path from user-controlled glob pattern, acceptable for session discovery
        if err != nil {
                return SessionMetadata{}, false, fmt.Errorf("open file: %w", err)
        }
        defer f.Close()

        reader := bufio.NewReader(f)

        for {
                line, readErr := reader.ReadString('\n')
                line = trimLineEnding(line)

                // stop at separator line; check readErr even when breaking
                if line != "" && strings.HasPrefix(line, "---") {
                        if readErr != nil && !errors.Is(readErr, io.EOF) {
                                return SessionMetadata{}, false, fmt.Errorf("read file: %w", readErr)
                        }
                        complete = true
                        break
                }

                // parse key-value pairs (process line before checking error,
                // as ReadString may return partial data alongside an error)
                if val, found := strings.CutPrefix(line, "Plan: "); found {
                        meta.PlanPath = val
                } else if val, found := strings.CutPrefix(line, "Branch: "); found {
                        meta.Branch = val
                } else if val, found := strings.CutPrefix(line, "Mode: "); found {
                        meta.Mode = val
                } else if val, found := strings.CutPrefix(line, "Started: "); found {
                        // header timestamps are written in local time without a zone offset
                        if t, parseErr := time.ParseInLocation("2006-01-02 15:04:05", val, time.Local); parseErr == nil {
                                meta.StartTime = t
                        }
                }

                if readErr != nil {
                        if !errors.Is(readErr, io.EOF) {
                                return SessionMetadata{}, false, fmt.Errorf("read file: %w", readErr)
                        }
                        break // EOF after processing final line
                }
        }

        return meta, complete, nil
}

// loadProgressFileIntoSession reads a progress file and publishes events to the session's SSE server.
// used for completed sessions that were discovered after they finished.
// errors are silently ignored since this is best-effort loading.
// records the total bytes consumed into session.lastOffset so a later Reactivate()
// can resume tailing from after the loaded content instead of re-emitting it.
func (m *SessionManager) loadProgressFileIntoSession(path string, session *Session) {
        f, err := os.Open(path) //nolint:gosec // path from user-controlled glob pattern, acceptable for session discovery
        if err != nil {
                return
        }
        defer f.Close()

        reader := bufio.NewReader(f)
        inHeader := true
        phase := status.PhaseTask
        var pendingSection string // section header waiting for first timestamped event
        var bytesRead int64

        for {
                line, readErr := reader.ReadString('\n')
                // a partial trailing line (no newline delimiter) means the writer is
                // mid-write — the flock-race recovery path is the realistic case where
                // loader is invoked on a still-being-written file. counting and
                // publishing the partial would advance lastOffset past the partial
                // bytes; a later Reactivate would then resume reading the suffix of
                // the writer's eventual completed line as if it were a separate event,
                // reintroducing the corruption this PR is meant to eliminate.
                if readErr != nil && line != "" {
                        break
                }
                // count raw bytes including the delimiter before trimming, so LF, CRLF,
                // and the final empty read all count correctly
                bytesRead += int64(len(line))
                line = trimLineEnding(line)

                if line != "" {
                        var parsed ParsedLine
                        parsed, inHeader = parseProgressLine(line, inHeader)
                        phase, pendingSection = m.processProgressLine(session, parsed, phase, pendingSection)
                }

                if readErr != nil {
                        break // EOF or read error; best-effort loading, errors silently ignored
                }
        }

        if pendingSection != "" {
                m.publishEvents(session, buildPendingSectionEvents(pendingSection, phase, time.Now()))
        }

        session.setLastOffset(bytesRead)
        // record the phase the parser ended on so a later Reactivate resumes with
        // the correct phase rather than the tailer's default (PhaseTask).
        session.setLastPhase(phase)
}

// processProgressLine handles a single parsed progress line,
// updating phase and pendingSection state and publishing events as needed.
func (m *SessionManager) processProgressLine(session *Session, parsed ParsedLine,
        phase status.Phase, pendingSection string) (status.Phase, string) {
        switch parsed.Type {
        case ParsedLineSkip:
                return phase, pendingSection
        case ParsedLineSection:
                if pendingSection != "" {
                        m.publishEvents(session, buildPendingSectionEvents(pendingSection, phase, time.Now()))
                }
                phase = parsed.Phase
                // defer emitting section until we see a timestamped event
                return phase, parsed.Section
        case ParsedLineTimestamp:
                // emit pending section with this event's timestamp (for accurate durations)
                if pendingSection != "" {
                        m.publishEvents(session, buildPendingSectionEvents(pendingSection, phase, parsed.Timestamp))
                        pendingSection = ""
                }
                event := eventFromParsed(parsed, phase)
                if event.Type == EventTypeOutput {
                        if stats, ok := parseDiffStats(event.Text); ok {
                                session.SetDiffStats(stats)
                        }
                }
                m.publishEvent(session, event)
        case ParsedLinePlain:
                m.publishEvent(session, eventFromParsed(parsed, phase))
        }
        return phase, pendingSection
}

func (m *SessionManager) publishEvents(session *Session, events []Event) {
        for _, event := range events {
                m.publishEvent(session, event)
        }
}

func (m *SessionManager) publishEvent(session *Session, event Event) {
        if err := session.Publish(event); err != nil {
                log.Printf("[WARN] failed to publish %s event: %v", event.Type, err)
        }
}

// phaseFromSection determines the phase from a section name.
// checks "codex"/"custom" before "review" because external review sections should be PhaseCodex.
func phaseFromSection(name string) status.Phase {
        nameLower := strings.ToLower(name)
        switch {
        case strings.Contains(nameLower, "task"):
                return status.PhaseTask
        case strings.Contains(nameLower, "codex"), strings.Contains(nameLower, "custom"):
                return status.PhaseCodex
        case strings.Contains(nameLower, "review"):
                return status.PhaseReview
        case strings.Contains(nameLower, "claude-eval"), strings.Contains(nameLower, "claude eval"):
                return status.PhaseClaudeEval
        default:
                return status.PhaseTask
        }
}

// trimLineEnding removes trailing line ending to match bufio.ScanLines semantics:
// strips \n, \r\n, or a bare trailing \r (which ScanLines drops via dropCR at EOF).
// unlike strings.TrimRight("\r\n"), this preserves embedded \r characters in content.
func trimLineEnding(line string) string {
        n := len(line)
        if n > 0 && line[n-1] == '\n' {
                n--
        }
        if n > 0 && line[n-1] == '\r' {
                n--
        }
        return line[:n]
}

1	package web
2
3	import (
4	"bufio"
5	"errors"
6	"fmt"
7	"io"
8	"log"
9	"os"
10	"strings"
11	"time"
12
13	"github.com/umputun/ralphex/pkg/status"
14	)
15
16	// ParseProgressHeader reads the header section of a progress file and extracts metadata.
17	// the header format is:
18	//
19	// # Ralphex Progress Log
20	// Plan: path/to/plan.md
21	// Branch: feature-branch
22	// Mode: full
23	// Started: 2026-01-22 10:30:00
24	// ------------------------------------------------------------
25	//
26	// the second return value reports whether the terminating separator line was
27	// observed, which means the header is fully written. during a truncate+rewrite
28	// the header is emitted across several writes, so a mid-write read can return
29	// incomplete metadata (zero StartTime, missing fields); callers should use the
30	// complete flag to decide whether to trust the parsed metadata.
31	func ParseProgressHeader(path string) (meta SessionMetadata, complete bool, err error) {	172✔
32	f, err := os.Open(path) //nolint:gosec // path from user-controlled glob pattern, acceptable for session discovery	172✔
33	if err != nil {	174✔
34	return SessionMetadata{}, false, fmt.Errorf("open file: %w", err)	2✔
35	}	2✔
36	defer f.Close()	170✔
37		170✔
38	reader := bufio.NewReader(f)	170✔
39		170✔
40	for {	1,183✔
41	line, readErr := reader.ReadString('\n')	1,013✔
42	line = trimLineEnding(line)	1,013✔
43		1,013✔
44	// stop at separator line; check readErr even when breaking	1,013✔
45	if line != "" && strings.HasPrefix(line, "---") {	1,180✔
46	if readErr != nil && !errors.Is(readErr, io.EOF) {	167✔
47	return SessionMetadata{}, false, fmt.Errorf("read file: %w", readErr)	×
48	}	×
49	complete = true	167✔
50	break	167✔
51	}
52
53	// parse key-value pairs (process line before checking error,
54	// as ReadString may return partial data alongside an error)
55	if val, found := strings.CutPrefix(line, "Plan: "); found {	1,014✔
56	meta.PlanPath = val	168✔
57	} else if val, found := strings.CutPrefix(line, "Branch: "); found {	1,016✔
58	meta.Branch = val	170✔
59	} else if val, found := strings.CutPrefix(line, "Mode: "); found {	847✔
60	meta.Mode = val	169✔
61	} else if val, found := strings.CutPrefix(line, "Started: "); found {	674✔
62	// header timestamps are written in local time without a zone offset	166✔
63	if t, parseErr := time.ParseInLocation("2006-01-02 15:04:05", val, time.Local); parseErr == nil {	332✔
64	meta.StartTime = t	166✔
65	}	166✔
66	}
67
68	if readErr != nil {	849✔
69	if !errors.Is(readErr, io.EOF) {	3✔
70	return SessionMetadata{}, false, fmt.Errorf("read file: %w", readErr)	×
71	}	×
72	break // EOF after processing final line	3✔
73	}
74	}
75
76	return meta, complete, nil	170✔
77	}
78
79	// loadProgressFileIntoSession reads a progress file and publishes events to the session's SSE server.
80	// used for completed sessions that were discovered after they finished.
81	// errors are silently ignored since this is best-effort loading.
82	// records the total bytes consumed into session.lastOffset so a later Reactivate()
83	// can resume tailing from after the loaded content instead of re-emitting it.
84	func (m SessionManager) loadProgressFileIntoSession(path string, session Session) {	164✔
85	f, err := os.Open(path) //nolint:gosec // path from user-controlled glob pattern, acceptable for session discovery	164✔
86	if err != nil {	166✔
87	return	2✔
88	}	2✔
89	defer f.Close()	162✔
90		162✔
91	reader := bufio.NewReader(f)	162✔
92	inHeader := true	162✔
93	phase := status.PhaseTask	162✔
94	var pendingSection string // section header waiting for first timestamped event	162✔
95	var bytesRead int64	162✔
96		162✔
97	for {	1,410✔
98	line, readErr := reader.ReadString('\n')	1,248✔
99	// a partial trailing line (no newline delimiter) means the writer is	1,248✔
100	// mid-write — the flock-race recovery path is the realistic case where	1,248✔
101	// loader is invoked on a still-being-written file. counting and	1,248✔
102	// publishing the partial would advance lastOffset past the partial	1,248✔
103	// bytes; a later Reactivate would then resume reading the suffix of	1,248✔
104	// the writer's eventual completed line as if it were a separate event,	1,248✔
105	// reintroducing the corruption this PR is meant to eliminate.	1,248✔
106	if readErr != nil && line != "" {	1,249✔
107	break	1✔
108	}
109	// count raw bytes including the delimiter before trimming, so LF, CRLF,
110	// and the final empty read all count correctly
111	bytesRead += int64(len(line))	1,247✔
112	line = trimLineEnding(line)	1,247✔
113		1,247✔
114	if line != "" {	2,288✔
115	var parsed ParsedLine	1,041✔
116	parsed, inHeader = parseProgressLine(line, inHeader)	1,041✔
117	phase, pendingSection = m.processProgressLine(session, parsed, phase, pendingSection)	1,041✔
118	}	1,041✔
119
120	if readErr != nil {	1,408✔
121	break // EOF or read error; best-effort loading, errors silently ignored	161✔
122	}
123	}
124
125	if pendingSection != "" {	163✔
126	m.publishEvents(session, buildPendingSectionEvents(pendingSection, phase, time.Now()))	1✔
127	}	1✔
128
129	session.setLastOffset(bytesRead)	162✔
130	// record the phase the parser ended on so a later Reactivate resumes with	162✔
131	// the correct phase rather than the tailer's default (PhaseTask).	162✔
132	session.setLastPhase(phase)	162✔
133	}
134
135	// processProgressLine handles a single parsed progress line,
136	// updating phase and pendingSection state and publishing events as needed.
137	func (m SessionManager) processProgressLine(session Session, parsed ParsedLine,
138	phase status.Phase, pendingSection string) (status.Phase, string) {	1,041✔
139	switch parsed.Type {	1,041✔
140	case ParsedLineSkip:	965✔
141	return phase, pendingSection	965✔
142	case ParsedLineSection:	16✔
143	if pendingSection != "" {	16✔
NEW 144	m.publishEvents(session, buildPendingSectionEvents(pendingSection, phase, time.Now()))	×
145	}	×
146	phase = parsed.Phase	16✔
147	// defer emitting section until we see a timestamped event	16✔
148	return phase, parsed.Section	16✔
149	case ParsedLineTimestamp:	58✔
150	// emit pending section with this event's timestamp (for accurate durations)	58✔
151	if pendingSection != "" {	73✔
152	m.publishEvents(session, buildPendingSectionEvents(pendingSection, phase, parsed.Timestamp))	15✔
153	pendingSection = ""	15✔
154	}	15✔
155	event := eventFromParsed(parsed, phase)	58✔
156	if event.Type == EventTypeOutput {	114✔
157	if stats, ok := parseDiffStats(event.Text); ok {	57✔
158	session.SetDiffStats(stats)	1✔
159	}	1✔
160	}
161	m.publishEvent(session, event)	58✔
162	case ParsedLinePlain:	2✔
163	m.publishEvent(session, eventFromParsed(parsed, phase))	2✔
164	}
165	return phase, pendingSection	60✔
166	}
167
168	func (m SessionManager) publishEvents(session Session, events []Event) {	16✔
169	for _, event := range events {	38✔
170	m.publishEvent(session, event)	22✔
171	}	22✔
172	}
173
174	func (m SessionManager) publishEvent(session Session, event Event) {	82✔
175	if err := session.Publish(event); err != nil {	82✔
NEW 176	log.Printf("[WARN] failed to publish %s event: %v", event.Type, err)	×
UNCOV 177	}	×
178	}
179
180	// phaseFromSection determines the phase from a section name.
181	// checks "codex"/"custom" before "review" because external review sections should be PhaseCodex.
182	func phaseFromSection(name string) status.Phase {	64✔
183	nameLower := strings.ToLower(name)	64✔
184	switch {	64✔
185	case strings.Contains(nameLower, "task"):	26✔
186	return status.PhaseTask	26✔
187	case strings.Contains(nameLower, "codex"), strings.Contains(nameLower, "custom"):	11✔
188	return status.PhaseCodex	11✔
189	case strings.Contains(nameLower, "review"):	17✔
190	return status.PhaseReview	17✔
191	case strings.Contains(nameLower, "claude-eval"), strings.Contains(nameLower, "claude eval"):	8✔
192	return status.PhaseClaudeEval	8✔
193	default:	2✔
194	return status.PhaseTask	2✔
195	}
196	}
197
198	// trimLineEnding removes trailing line ending to match bufio.ScanLines semantics:
199	// strips \n, \r\n, or a bare trailing \r (which ScanLines drops via dropCR at EOF).
200	// unlike strings.TrimRight("\r\n"), this preserves embedded \r characters in content.
201	func trimLineEnding(line string) string {	2,269✔
202	n := len(line)	2,269✔
203	if n > 0 && line[n-1] == '\n' {	4,370✔
204	n--	2,101✔
205	}	2,101✔
206	if n > 0 && line[n-1] == '\r' {	2,283✔
207	n--	14✔
208	}	14✔
209	return line[:n]	2,269✔
210	}

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