diff --git a/pkg/codingcontext/context.go b/pkg/codingcontext/context.go index 47a7ba4b..cd4b198e 100644 --- a/pkg/codingcontext/context.go +++ b/pkg/codingcontext/context.go @@ -292,7 +292,7 @@ func (cc *Context) makeMarkdownWalkFunc(visitor markdownVisitor) filepath.WalkFu } var fm markdown.BaseFrontMatter - if _, parseErr := markdown.ParseMarkdownFile(path, &fm); parseErr != nil { + if _, parseErr := markdown.ParseMarkdownFileWithLogger(path, &fm, cc.logger); parseErr != nil { if cc.lintCollector != nil { var pe *markdown.ParseError if errors.As(parseErr, &pe) { @@ -386,7 +386,7 @@ func (cc *Context) findTask(taskName string) error { func (cc *Context) loadTask(path, taskName string) error { var frontMatter markdown.TaskFrontMatter - md, err := markdown.ParseMarkdownFile(path, &frontMatter) + md, err := markdown.ParseMarkdownFileWithLogger(path, &frontMatter, cc.logger) if err != nil { return fmt.Errorf("failed to parse task file %s: %w", path, err) } @@ -428,7 +428,7 @@ func (cc *Context) loadTask(path, taskName string) error { var parseErr error - task, parseErr = taskparser.ParseTask(taskContent) + task, parseErr = taskparser.ParseTaskWithLogger(taskContent, cc.logger) if parseErr != nil { return fmt.Errorf("failed to parse task content in file %s: %w", path, parseErr) } @@ -549,7 +549,7 @@ func (cc *Context) findCommand(commandName string, params taskparser.Params) (st var frontMatter markdown.CommandFrontMatter - md, err := markdown.ParseMarkdownFile(path, &frontMatter) + md, err := markdown.ParseMarkdownFileWithLogger(path, &frontMatter, cc.logger) if err != nil { return fmt.Errorf("failed to parse command file %s: %w", path, err) } @@ -816,7 +816,7 @@ func (cc *Context) findExecuteRuleFiles(ctx context.Context) error { err := cc.visitMarkdownFiles(namespacedRulePaths, func(path string, baseFm *markdown.BaseFrontMatter) error { var frontmatter markdown.RuleFrontMatter - md, err := markdown.ParseMarkdownFile(path, &frontmatter) + md, err := markdown.ParseMarkdownFileWithLogger(path, &frontmatter, cc.logger) if err != nil { return fmt.Errorf("failed to parse markdown file %s: %w", path, err) } @@ -1042,7 +1042,7 @@ func (cc *Context) loadSkillEntry(skillFile string, lenient bool) error { var frontmatter markdown.SkillFrontMatter - if _, err := markdown.ParseMarkdownFile(skillFile, &frontmatter); err != nil { + if _, err := markdown.ParseMarkdownFileWithLogger(skillFile, &frontmatter, cc.logger); err != nil { if lenient { cc.logger.Warn("skipping skill file: failed to parse YAML frontmatter", "path", skillFile, "error", err) diff --git a/pkg/codingcontext/markdown/markdown.go b/pkg/codingcontext/markdown/markdown.go index 8c1f21b2..42a60d3c 100644 --- a/pkg/codingcontext/markdown/markdown.go +++ b/pkg/codingcontext/markdown/markdown.go @@ -3,6 +3,7 @@ package markdown import ( "bytes" "fmt" + "log/slog" "os" "path/filepath" "strings" @@ -68,7 +69,16 @@ type RuleMarkdown = Markdown[RuleFrontMatter] // ParseMarkdownFile parses a markdown file into frontmatter and content using goldmark. // Errors include file path and, where available, line and column position. +// Uses slog.Default() for taskparser WARN logs; for a custom logger use +// ParseMarkdownFileWithLogger. func ParseMarkdownFile[T any](path string, frontMatter *T) (Markdown[T], error) { + return ParseMarkdownFileWithLogger(path, frontMatter, nil) +} + +// ParseMarkdownFileWithLogger is like ParseMarkdownFile but routes taskparser +// best-effort fallback WARN logs through the given logger. A nil logger +// resolves to slog.Default() at parse time (not capture time). +func ParseMarkdownFileWithLogger[T any](path string, frontMatter *T, logger *slog.Logger) (Markdown[T], error) { cleanPath := filepath.Clean(path) source, err := os.ReadFile(cleanPath) @@ -77,9 +87,9 @@ func ParseMarkdownFile[T any](path string, frontMatter *T) (Markdown[T], error) } // Parse with goldmark+meta+taskparser in a single pass: meta extracts frontmatter, - // taskparser.Extension captures task structure (slash commands) from the body. + // the taskparser extension captures task structure (slash commands) from the body. pctx := parser.NewContext() - doc := goldmark.New(goldmark.WithExtensions(meta.Meta, taskparser.Extension)).Parser(). + doc := goldmark.New(goldmark.WithExtensions(meta.Meta, taskparser.NewExtension(logger))).Parser(). Parse(text.NewReader(source), parser.WithContext(pctx)) // Get frontmatter map from goldmark-meta (parsed during goldmark parse). diff --git a/pkg/codingcontext/taskparser/markdownparser.go b/pkg/codingcontext/taskparser/markdownparser.go index 0cb91a1d..f35301b5 100644 --- a/pkg/codingcontext/taskparser/markdownparser.go +++ b/pkg/codingcontext/taskparser/markdownparser.go @@ -3,6 +3,7 @@ package taskparser import ( "bytes" "fmt" + "log/slog" "sort" "strings" @@ -95,14 +96,14 @@ func collectCodeRanges(doc ast.Node) ([]codeRange, error) { // newline characters. This prevents the next text segment from starting with a bare // newline immediately before a slash, which would cause the grammar parser to fail // (it cannot parse a Text block that has only leading newlines before a slash). -func splitAndParse(content string, codeRanges []codeRange) (Task, error) { +func splitAndParse(content string, codeRanges []codeRange, logger *slog.Logger) (Task, error) { var allBlocks []Block pos := 0 for i, cr := range codeRanges { if pos < cr.start { - blocks, err := parseGrammar(content[pos:cr.start]) + blocks, err := parseGrammar(content[pos:cr.start], logger) if err != nil { return nil, err } @@ -127,7 +128,7 @@ func splitAndParse(content string, codeRanges []codeRange) (Task, error) { } if pos < len(content) { - blocks, err := parseGrammar(content[pos:]) + blocks, err := parseGrammar(content[pos:], logger) if err != nil { return nil, err } @@ -149,17 +150,63 @@ func trailingNewlineEnd(content string, pos int) int { // parseGrammar runs the participle grammar parser on a plain text segment. // It returns nil blocks (not an error) for whitespace-only input. -func parseGrammar(content string) ([]Block, error) { +// +// On parser failure (e.g. a line starting with `/` but lacking a valid term: +// `//`, lone `/`, `/=foo`), falls back to a best-effort line-by-line parse so +// one malformed line cannot discard the entire task. +func parseGrammar(content string, logger *slog.Logger) ([]Block, error) { if strings.TrimSpace(content) == "" { return nil, nil } input, err := parser().ParseString("", content) - if err != nil { - return nil, fmt.Errorf("failed to parse task: %w", err) + if err == nil { + return input.Blocks, nil + } + + return parseGrammarBestEffort(content, logger), nil +} + +// loggerOrDefault returns logger, or slog.Default() if logger is nil. Resolving +// at use time (rather than capturing at construction) ensures slog.SetDefault +// changes take effect for callers that pass nil. +func loggerOrDefault(logger *slog.Logger) *slog.Logger { + if logger == nil { + return slog.Default() } - return input.Blocks, nil + return logger +} + +// parseGrammarBestEffort parses content line-by-line, returning a raw text +// block for any line the grammar rejects. Called only after the whole-segment +// parse has already failed. +func parseGrammarBestEffort(content string, logger *slog.Logger) []Block { + var blocks []Block + + for _, line := range strings.SplitAfter(content, "\n") { + if line == "" { + continue // strings.SplitAfter emits a trailing "" when input ends with "\n" + } + + if strings.TrimSpace(line) == "" { + blocks = append(blocks, rawTextBlock(line)) + continue + } + + input, err := parser().ParseString("", line) + if err != nil { + loggerOrDefault(logger).Warn("taskparser: malformed slash command, treating line as text", + "line", strings.TrimRight(line, "\r\n"), "error", err) + blocks = append(blocks, rawTextBlock(line)) + + continue + } + + blocks = append(blocks, input.Blocks...) + } + + return blocks } // rawTextBlock wraps a raw string as a Text block without any slash command parsing. @@ -176,6 +223,8 @@ func rawTextBlock(content string) Block { // Extension is a goldmark extension that parses task structure during the markdown parse. // Include it in a goldmark instance and use GetTask to retrieve the parsed Task after parsing. +// Uses slog.Default() for WARN logs from the best-effort fallback; for a custom logger +// use NewExtension. // // Example: // @@ -185,14 +234,23 @@ func rawTextBlock(content string) Block { // task, err := taskparser.GetTask(pctx) // //nolint:gochecknoglobals // goldmark.WithExtensions expects a package-level extender -var Extension goldmark.Extender = &taskExtension{} +var Extension goldmark.Extender = NewExtension(nil) -type taskExtension struct{} +// NewExtension returns a goldmark extension that parses task structure and routes +// best-effort fallback WARN logs through the given logger. A nil logger resolves to +// slog.Default() at parse time (not capture time), so SetDefault changes take effect. +func NewExtension(logger *slog.Logger) goldmark.Extender { + return &taskExtension{logger: logger} +} + +type taskExtension struct { + logger *slog.Logger // nil means use slog.Default() at parse time +} func (e *taskExtension) Extend(m goldmark.Markdown) { const taskTransformerPriority = 100 m.Parser().AddOptions(gparser.WithASTTransformers( - util.Prioritized(&taskTransformer{}, taskTransformerPriority), + util.Prioritized(&taskTransformer{logger: e.logger}, taskTransformerPriority), )) } @@ -225,7 +283,9 @@ func GetTask(pc gparser.Context) (Task, error) { // taskTransformer implements parser.ASTTransformer. It runs after goldmark has built // the document AST and extracts task structure (text vs. slash commands), skipping // slash command detection inside code blocks, indented code, and HTML blocks. -type taskTransformer struct{} +type taskTransformer struct { + logger *slog.Logger +} func (t *taskTransformer) Transform(node *ast.Document, reader text.Reader, pc gparser.Context) { source := reader.Source() @@ -265,17 +325,17 @@ func (t *taskTransformer) Transform(node *ast.Document, reader text.Reader, pc g adjusted = append(adjusted, codeRange{adjStart, adjStop}) } - task, parseErr := splitAndParse(content, adjusted) + task, parseErr := splitAndParse(content, adjusted, t.logger) pc.Set(contextKey, &taskParseResult{task: task, err: parseErr}) } // parseMarkdownAware parses task content while skipping slash command detection inside // code blocks (fenced code, indented code, HTML blocks) by running the Extension -// during a single goldmark parse pass. -func parseMarkdownAware(content string) (Task, error) { +// during a single goldmark parse pass. A nil logger falls back to slog.Default(). +func parseMarkdownAware(content string, logger *slog.Logger) (Task, error) { source := []byte(content) pctx := gparser.NewContext() - goldmark.New(goldmark.WithExtensions(Extension)).Parser(). + goldmark.New(goldmark.WithExtensions(NewExtension(logger))).Parser(). Parse(text.NewReader(source), gparser.WithContext(pctx)) return GetTask(pctx) diff --git a/pkg/codingcontext/taskparser/taskparser.go b/pkg/codingcontext/taskparser/taskparser.go index 9ea37abf..faedde38 100644 --- a/pkg/codingcontext/taskparser/taskparser.go +++ b/pkg/codingcontext/taskparser/taskparser.go @@ -2,6 +2,7 @@ package taskparser import ( + "log/slog" "strings" ) @@ -111,13 +112,20 @@ import ( // task, _ := ParseTask("Introduction text\n/fix-bug 123\nSome text after") // // len(task) == 3 (text, command, text) func ParseTask(text string) (Task, error) { + return ParseTaskWithLogger(text, nil) +} + +// ParseTaskWithLogger is like ParseTask but routes best-effort fallback WARN +// logs through the given logger. A nil logger resolves to slog.Default() at +// parse time (not capture time), so SetDefault changes take effect. +func ParseTaskWithLogger(text string, logger *slog.Logger) (Task, error) { // Handle empty or whitespace-only content gracefully // TrimSpace returns empty string for whitespace-only input if strings.TrimSpace(text) == "" { return Task{}, nil } - return parseMarkdownAware(text) + return parseMarkdownAware(text, logger) } // Params converts the slash command's arguments into a parameter map using ParseParams. diff --git a/pkg/codingcontext/taskparser/taskparser_test.go b/pkg/codingcontext/taskparser/taskparser_test.go index 85b907df..20377618 100644 --- a/pkg/codingcontext/taskparser/taskparser_test.go +++ b/pkg/codingcontext/taskparser/taskparser_test.go @@ -1,6 +1,8 @@ package taskparser import ( + "bytes" + "log/slog" "strings" "testing" ) @@ -465,6 +467,117 @@ func markdownAwareTestCases() []parseTaskCase { } } +// checkTextContains asserts the parsed task round-trips so the given substring +// is present in the reconstructed content. Used for "best-effort" cases where a +// malformed slash-command line must be preserved as text rather than aborting parse. +func checkTextContains(needles ...string) func(t *testing.T, task Task) { + return func(t *testing.T, task Task) { + t.Helper() + + got := task.String() + for _, needle := range needles { + if !strings.Contains(got, needle) { + t.Errorf("expected reconstructed task to contain %q, got %q", needle, got) + } + } + } +} + +// Malformed slash-command lines outside a code block must not abort the parse. +// The line is preserved as text. These cases currently fail with +// "failed to parse task: ... unexpected token ..." because the participle +// grammar requires Slash @Term ... and treats // (and lone /) as a hard error, +// discarding the entire task. +func bestEffortMalformedSlashCases() []parseTaskCase { + return []parseTaskCase{ + { + // Also asserts "More text" survives — guards against a fix that parses + // line-by-line and salvages only the malformed line, dropping the + // surrounding text after it. + name: "best-effort: double slash bare line outside code block", + input: "Some text\n// Just a stray double slash\nMore text\n", + check: checkTextContains("// Just a stray double slash", "More text"), + }, + { + name: "best-effort: lone slash line outside code block", + input: "Intro\n/\nMore text\n", + check: checkTextContains("Intro"), + }, + { + name: "best-effort: slash followed by non-term char outside code block", + input: "Intro\n/=oops\nMore text\n", + check: checkTextContains("Intro"), + }, + } +} + +func TestParseTask_BestEffortMalformedSlash(t *testing.T) { + t.Parallel() + + for _, tt := range bestEffortMalformedSlashCases() { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + task, err := ParseTask(tt.input) + if err != nil { + t.Fatalf("ParseTask() error = %v, want nil (malformed slash lines must not abort parse)", err) + } + + if tt.check != nil { + tt.check(t, task) + } + }) + } +} + +// Verifies ParseTaskWithLogger routes the best-effort fallback WARN through +// the injected logger rather than slog.Default(), so callers can capture or +// redirect parser warnings. +func TestParseTaskWithLogger_RoutesWarnToInjectedLogger(t *testing.T) { + t.Parallel() + + var buf bytes.Buffer + logger := slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn})) + + input := "Some text\n// stray\nMore text\n" + + if _, err := ParseTaskWithLogger(input, logger); err != nil { + t.Fatalf("ParseTaskWithLogger() error = %v, want nil", err) + } + + out := buf.String() + if !strings.Contains(out, "malformed slash command") { + t.Errorf("expected WARN about malformed slash command in injected logger output, got %q", out) + } + + if !strings.Contains(out, "// stray") { + t.Errorf("expected WARN to include the offending line, got %q", out) + } +} + +// Verifies a nil logger resolves to slog.Default() at parse time, not capture +// time — so SetDefault changes made after the package loads still take effect. +// Guards against a regression where the default is snapshotted at construction. +func TestParseTaskWithLogger_NilLoggerResolvesLazyDefault(t *testing.T) { + // Not t.Parallel: this test mutates slog's package-level default. + originalDefault := slog.Default() + + t.Cleanup(func() { slog.SetDefault(originalDefault) }) + + var buf bytes.Buffer + slog.SetDefault(slog.New(slog.NewTextHandler(&buf, &slog.HandlerOptions{Level: slog.LevelWarn}))) + + input := "Some text\n// stray\nMore text\n" + + if _, err := ParseTaskWithLogger(input, nil); err != nil { + t.Fatalf("ParseTaskWithLogger(nil) error = %v, want nil", err) + } + + if !strings.Contains(buf.String(), "malformed slash command") { + t.Errorf("expected WARN to be routed through the newly-set default logger, got %q", buf.String()) + } +} + func TestParseTask_MarkdownAware(t *testing.T) { t.Parallel()