Skip to content

Commit 8eaf28b

Browse files
authored
fix: best-effort fallback when slash line is invalid grammar (#217)
* test: add failing cases for best-effort malformed slash lines ParseTask currently aborts the entire parse when a line starts with `/` but isn't a valid slash command (e.g. `//`, lone `/`, `/=foo`). These should fall through to text, preserving both the malformed line and any surrounding content. In-fence slashes are already handled by the markdown-aware path; only the bare-text case is broken. * fix: best-effort fallback when slash line is invalid grammar A single malformed slash line (e.g. `//`, lone `/`, `/=foo`) outside a fenced code block caused ParseTask to abort the entire task, silently dropping the user prompt and frontmatter and corrupting downstream runs. On parser failure, parseGrammar now re-attempts line-by-line. Lines that still fail are emitted as raw text with a WARN logged at slog.Default. The whole-segment fast path is unchanged for valid input. * feat: inject logger through markdown and taskparser parse APIs Adds *WithLogger variants for the parse entry points so callers can route taskparser's best-effort fallback WARN logs through their own slog.Logger instead of slog.Default(): - taskparser.NewExtension(logger) (Extension stays as NewExtension(nil)) - taskparser.ParseTaskWithLogger(text, logger) - markdown.ParseMarkdownFileWithLogger(path, fm, logger) The original ParseTask/ParseMarkdownFile/Extension entry points are preserved as nil-logger wrappers, so external callers are unaffected. A nil logger resolves to slog.Default() at log time (via loggerOrDefault) rather than being captured at construction, so slog.SetDefault changes made after package load still take effect. A new test covers this. All five ParseMarkdownFile callsites and the ParseTask callsite in pkg/codingcontext/context.go now pass cc.logger.
1 parent 4b1d82d commit 8eaf28b

5 files changed

Lines changed: 215 additions & 24 deletions

File tree

pkg/codingcontext/context.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -292,7 +292,7 @@ func (cc *Context) makeMarkdownWalkFunc(visitor markdownVisitor) filepath.WalkFu
292292
}
293293

294294
var fm markdown.BaseFrontMatter
295-
if _, parseErr := markdown.ParseMarkdownFile(path, &fm); parseErr != nil {
295+
if _, parseErr := markdown.ParseMarkdownFileWithLogger(path, &fm, cc.logger); parseErr != nil {
296296
if cc.lintCollector != nil {
297297
var pe *markdown.ParseError
298298
if errors.As(parseErr, &pe) {
@@ -386,7 +386,7 @@ func (cc *Context) findTask(taskName string) error {
386386
func (cc *Context) loadTask(path, taskName string) error {
387387
var frontMatter markdown.TaskFrontMatter
388388

389-
md, err := markdown.ParseMarkdownFile(path, &frontMatter)
389+
md, err := markdown.ParseMarkdownFileWithLogger(path, &frontMatter, cc.logger)
390390
if err != nil {
391391
return fmt.Errorf("failed to parse task file %s: %w", path, err)
392392
}
@@ -428,7 +428,7 @@ func (cc *Context) loadTask(path, taskName string) error {
428428

429429
var parseErr error
430430

431-
task, parseErr = taskparser.ParseTask(taskContent)
431+
task, parseErr = taskparser.ParseTaskWithLogger(taskContent, cc.logger)
432432
if parseErr != nil {
433433
return fmt.Errorf("failed to parse task content in file %s: %w", path, parseErr)
434434
}
@@ -549,7 +549,7 @@ func (cc *Context) findCommand(commandName string, params taskparser.Params) (st
549549

550550
var frontMatter markdown.CommandFrontMatter
551551

552-
md, err := markdown.ParseMarkdownFile(path, &frontMatter)
552+
md, err := markdown.ParseMarkdownFileWithLogger(path, &frontMatter, cc.logger)
553553
if err != nil {
554554
return fmt.Errorf("failed to parse command file %s: %w", path, err)
555555
}
@@ -816,7 +816,7 @@ func (cc *Context) findExecuteRuleFiles(ctx context.Context) error {
816816
err := cc.visitMarkdownFiles(namespacedRulePaths, func(path string, baseFm *markdown.BaseFrontMatter) error {
817817
var frontmatter markdown.RuleFrontMatter
818818

819-
md, err := markdown.ParseMarkdownFile(path, &frontmatter)
819+
md, err := markdown.ParseMarkdownFileWithLogger(path, &frontmatter, cc.logger)
820820
if err != nil {
821821
return fmt.Errorf("failed to parse markdown file %s: %w", path, err)
822822
}
@@ -1042,7 +1042,7 @@ func (cc *Context) loadSkillEntry(skillFile string, lenient bool) error {
10421042

10431043
var frontmatter markdown.SkillFrontMatter
10441044

1045-
if _, err := markdown.ParseMarkdownFile(skillFile, &frontmatter); err != nil {
1045+
if _, err := markdown.ParseMarkdownFileWithLogger(skillFile, &frontmatter, cc.logger); err != nil {
10461046
if lenient {
10471047
cc.logger.Warn("skipping skill file: failed to parse YAML frontmatter", "path", skillFile, "error", err)
10481048

pkg/codingcontext/markdown/markdown.go

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package markdown
33
import (
44
"bytes"
55
"fmt"
6+
"log/slog"
67
"os"
78
"path/filepath"
89
"strings"
@@ -68,7 +69,16 @@ type RuleMarkdown = Markdown[RuleFrontMatter]
6869

6970
// ParseMarkdownFile parses a markdown file into frontmatter and content using goldmark.
7071
// Errors include file path and, where available, line and column position.
72+
// Uses slog.Default() for taskparser WARN logs; for a custom logger use
73+
// ParseMarkdownFileWithLogger.
7174
func ParseMarkdownFile[T any](path string, frontMatter *T) (Markdown[T], error) {
75+
return ParseMarkdownFileWithLogger(path, frontMatter, nil)
76+
}
77+
78+
// ParseMarkdownFileWithLogger is like ParseMarkdownFile but routes taskparser
79+
// best-effort fallback WARN logs through the given logger. A nil logger
80+
// resolves to slog.Default() at parse time (not capture time).
81+
func ParseMarkdownFileWithLogger[T any](path string, frontMatter *T, logger *slog.Logger) (Markdown[T], error) {
7282
cleanPath := filepath.Clean(path)
7383

7484
source, err := os.ReadFile(cleanPath)
@@ -77,9 +87,9 @@ func ParseMarkdownFile[T any](path string, frontMatter *T) (Markdown[T], error)
7787
}
7888

7989
// Parse with goldmark+meta+taskparser in a single pass: meta extracts frontmatter,
80-
// taskparser.Extension captures task structure (slash commands) from the body.
90+
// the taskparser extension captures task structure (slash commands) from the body.
8191
pctx := parser.NewContext()
82-
doc := goldmark.New(goldmark.WithExtensions(meta.Meta, taskparser.Extension)).Parser().
92+
doc := goldmark.New(goldmark.WithExtensions(meta.Meta, taskparser.NewExtension(logger))).Parser().
8393
Parse(text.NewReader(source), parser.WithContext(pctx))
8494

8595
// Get frontmatter map from goldmark-meta (parsed during goldmark parse).

pkg/codingcontext/taskparser/markdownparser.go

Lines changed: 75 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package taskparser
33
import (
44
"bytes"
55
"fmt"
6+
"log/slog"
67
"sort"
78
"strings"
89

@@ -95,14 +96,14 @@ func collectCodeRanges(doc ast.Node) ([]codeRange, error) {
9596
// newline characters. This prevents the next text segment from starting with a bare
9697
// newline immediately before a slash, which would cause the grammar parser to fail
9798
// (it cannot parse a Text block that has only leading newlines before a slash).
98-
func splitAndParse(content string, codeRanges []codeRange) (Task, error) {
99+
func splitAndParse(content string, codeRanges []codeRange, logger *slog.Logger) (Task, error) {
99100
var allBlocks []Block
100101

101102
pos := 0
102103

103104
for i, cr := range codeRanges {
104105
if pos < cr.start {
105-
blocks, err := parseGrammar(content[pos:cr.start])
106+
blocks, err := parseGrammar(content[pos:cr.start], logger)
106107
if err != nil {
107108
return nil, err
108109
}
@@ -127,7 +128,7 @@ func splitAndParse(content string, codeRanges []codeRange) (Task, error) {
127128
}
128129

129130
if pos < len(content) {
130-
blocks, err := parseGrammar(content[pos:])
131+
blocks, err := parseGrammar(content[pos:], logger)
131132
if err != nil {
132133
return nil, err
133134
}
@@ -149,17 +150,63 @@ func trailingNewlineEnd(content string, pos int) int {
149150

150151
// parseGrammar runs the participle grammar parser on a plain text segment.
151152
// It returns nil blocks (not an error) for whitespace-only input.
152-
func parseGrammar(content string) ([]Block, error) {
153+
//
154+
// On parser failure (e.g. a line starting with `/` but lacking a valid term:
155+
// `//`, lone `/`, `/=foo`), falls back to a best-effort line-by-line parse so
156+
// one malformed line cannot discard the entire task.
157+
func parseGrammar(content string, logger *slog.Logger) ([]Block, error) {
153158
if strings.TrimSpace(content) == "" {
154159
return nil, nil
155160
}
156161

157162
input, err := parser().ParseString("", content)
158-
if err != nil {
159-
return nil, fmt.Errorf("failed to parse task: %w", err)
163+
if err == nil {
164+
return input.Blocks, nil
165+
}
166+
167+
return parseGrammarBestEffort(content, logger), nil
168+
}
169+
170+
// loggerOrDefault returns logger, or slog.Default() if logger is nil. Resolving
171+
// at use time (rather than capturing at construction) ensures slog.SetDefault
172+
// changes take effect for callers that pass nil.
173+
func loggerOrDefault(logger *slog.Logger) *slog.Logger {
174+
if logger == nil {
175+
return slog.Default()
160176
}
161177

162-
return input.Blocks, nil
178+
return logger
179+
}
180+
181+
// parseGrammarBestEffort parses content line-by-line, returning a raw text
182+
// block for any line the grammar rejects. Called only after the whole-segment
183+
// parse has already failed.
184+
func parseGrammarBestEffort(content string, logger *slog.Logger) []Block {
185+
var blocks []Block
186+
187+
for _, line := range strings.SplitAfter(content, "\n") {
188+
if line == "" {
189+
continue // strings.SplitAfter emits a trailing "" when input ends with "\n"
190+
}
191+
192+
if strings.TrimSpace(line) == "" {
193+
blocks = append(blocks, rawTextBlock(line))
194+
continue
195+
}
196+
197+
input, err := parser().ParseString("", line)
198+
if err != nil {
199+
loggerOrDefault(logger).Warn("taskparser: malformed slash command, treating line as text",
200+
"line", strings.TrimRight(line, "\r\n"), "error", err)
201+
blocks = append(blocks, rawTextBlock(line))
202+
203+
continue
204+
}
205+
206+
blocks = append(blocks, input.Blocks...)
207+
}
208+
209+
return blocks
163210
}
164211

165212
// rawTextBlock wraps a raw string as a Text block without any slash command parsing.
@@ -176,6 +223,8 @@ func rawTextBlock(content string) Block {
176223

177224
// Extension is a goldmark extension that parses task structure during the markdown parse.
178225
// Include it in a goldmark instance and use GetTask to retrieve the parsed Task after parsing.
226+
// Uses slog.Default() for WARN logs from the best-effort fallback; for a custom logger
227+
// use NewExtension.
179228
//
180229
// Example:
181230
//
@@ -185,14 +234,23 @@ func rawTextBlock(content string) Block {
185234
// task, err := taskparser.GetTask(pctx)
186235
//
187236
//nolint:gochecknoglobals // goldmark.WithExtensions expects a package-level extender
188-
var Extension goldmark.Extender = &taskExtension{}
237+
var Extension goldmark.Extender = NewExtension(nil)
189238

190-
type taskExtension struct{}
239+
// NewExtension returns a goldmark extension that parses task structure and routes
240+
// best-effort fallback WARN logs through the given logger. A nil logger resolves to
241+
// slog.Default() at parse time (not capture time), so SetDefault changes take effect.
242+
func NewExtension(logger *slog.Logger) goldmark.Extender {
243+
return &taskExtension{logger: logger}
244+
}
245+
246+
type taskExtension struct {
247+
logger *slog.Logger // nil means use slog.Default() at parse time
248+
}
191249

192250
func (e *taskExtension) Extend(m goldmark.Markdown) {
193251
const taskTransformerPriority = 100
194252
m.Parser().AddOptions(gparser.WithASTTransformers(
195-
util.Prioritized(&taskTransformer{}, taskTransformerPriority),
253+
util.Prioritized(&taskTransformer{logger: e.logger}, taskTransformerPriority),
196254
))
197255
}
198256

@@ -225,7 +283,9 @@ func GetTask(pc gparser.Context) (Task, error) {
225283
// taskTransformer implements parser.ASTTransformer. It runs after goldmark has built
226284
// the document AST and extracts task structure (text vs. slash commands), skipping
227285
// slash command detection inside code blocks, indented code, and HTML blocks.
228-
type taskTransformer struct{}
286+
type taskTransformer struct {
287+
logger *slog.Logger
288+
}
229289

230290
func (t *taskTransformer) Transform(node *ast.Document, reader text.Reader, pc gparser.Context) {
231291
source := reader.Source()
@@ -265,17 +325,17 @@ func (t *taskTransformer) Transform(node *ast.Document, reader text.Reader, pc g
265325
adjusted = append(adjusted, codeRange{adjStart, adjStop})
266326
}
267327

268-
task, parseErr := splitAndParse(content, adjusted)
328+
task, parseErr := splitAndParse(content, adjusted, t.logger)
269329
pc.Set(contextKey, &taskParseResult{task: task, err: parseErr})
270330
}
271331

272332
// parseMarkdownAware parses task content while skipping slash command detection inside
273333
// code blocks (fenced code, indented code, HTML blocks) by running the Extension
274-
// during a single goldmark parse pass.
275-
func parseMarkdownAware(content string) (Task, error) {
334+
// during a single goldmark parse pass. A nil logger falls back to slog.Default().
335+
func parseMarkdownAware(content string, logger *slog.Logger) (Task, error) {
276336
source := []byte(content)
277337
pctx := gparser.NewContext()
278-
goldmark.New(goldmark.WithExtensions(Extension)).Parser().
338+
goldmark.New(goldmark.WithExtensions(NewExtension(logger))).Parser().
279339
Parse(text.NewReader(source), gparser.WithContext(pctx))
280340

281341
return GetTask(pctx)

pkg/codingcontext/taskparser/taskparser.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
package taskparser
33

44
import (
5+
"log/slog"
56
"strings"
67
)
78

@@ -111,13 +112,20 @@ import (
111112
// task, _ := ParseTask("Introduction text\n/fix-bug 123\nSome text after")
112113
// // len(task) == 3 (text, command, text)
113114
func ParseTask(text string) (Task, error) {
115+
return ParseTaskWithLogger(text, nil)
116+
}
117+
118+
// ParseTaskWithLogger is like ParseTask but routes best-effort fallback WARN
119+
// logs through the given logger. A nil logger resolves to slog.Default() at
120+
// parse time (not capture time), so SetDefault changes take effect.
121+
func ParseTaskWithLogger(text string, logger *slog.Logger) (Task, error) {
114122
// Handle empty or whitespace-only content gracefully
115123
// TrimSpace returns empty string for whitespace-only input
116124
if strings.TrimSpace(text) == "" {
117125
return Task{}, nil
118126
}
119127

120-
return parseMarkdownAware(text)
128+
return parseMarkdownAware(text, logger)
121129
}
122130

123131
// Params converts the slash command's arguments into a parameter map using ParseParams.

0 commit comments

Comments
 (0)