diff --git a/docs/reference/search-paths.md b/docs/reference/search-paths.md index 84281ae..2c6d529 100644 --- a/docs/reference/search-paths.md +++ b/docs/reference/search-paths.md @@ -298,6 +298,24 @@ For each rule file `rule-name.md`, the CLI looks for `rule-name-bootstrap` in th ./jira-context-bootstrap ← Wrong directory ``` +### Bootstrap failure handling + +Rule files are discovered and expanded for one rule directory before bootstrap +scripts from that directory run. Bootstrap processing completes before discovery +moves to the next rule directory, preserving search-path order. + +- **Strict search paths:** A rule bootstrap failure is fatal and `Run` returns an + error. Rules after the failing bootstrap are not published. A rule parse or + expansion error is also fatal and `Run` returns an error before any bootstrap + scripts from that directory run. +- **Lenient search paths:** A rule bootstrap failure logs a warning, excludes only + the failing rule, and continues with later rules in the same directory. + +A rule parse or expansion error still stops discovery of the remainder of that +directory. On a lenient path, rules discovered before that error remain eligible +for bootstrap and publication, and processing continues with the next rule +directory or search path. + ## Working Directory The `-C` option changes the working directory, which is automatically added to the search paths: diff --git a/pkg/codingcontext/context.go b/pkg/codingcontext/context.go index cd4b198..3e24192 100644 --- a/pkg/codingcontext/context.go +++ b/pkg/codingcontext/context.go @@ -116,11 +116,11 @@ func parseNamespacedTaskName(taskName string) (string, string, error) { // New creates a new Context with the given options. func New(opts ...Option) *Context { c := &Context{ - params: make(taskparser.Params), - includes: make(selectors.Selectors), - rules: make([]markdown.Markdown[markdown.RuleFrontMatter], 0), - skills: skills.AvailableSkills{Skills: make([]skills.Skill, 0)}, - logger: slog.New(slog.NewTextHandler(os.Stderr, nil)), + params: make(taskparser.Params), + includes: make(selectors.Selectors), + rules: make([]markdown.Markdown[markdown.RuleFrontMatter], 0), + skills: skills.AvailableSkills{Skills: make([]skills.Skill, 0)}, + logger: slog.New(slog.NewTextHandler(os.Stderr, nil)), doBootstrap: true, // Default to true for backward compatibility includeByDefault: true, // Default to true for backward compatibility cmdRunner: func(cmd *exec.Cmd) error { @@ -144,6 +144,28 @@ func nameFromPath(path string) string { type markdownVisitor func(path string, fm *markdown.BaseFrontMatter) error +// A ruleFileError names the rule file involved in a discovery or bootstrap failure. +type ruleFileError struct { + path string + err error +} + +func (e *ruleFileError) Error() string { + return fmt.Sprintf("%s: %s", e.path, e.err) +} + +func (e *ruleFileError) Unwrap() error { + return e.err +} + +// pendingRule is a parsed, expanded rule that has not been bootstrapped yet. +type pendingRule struct { + path string + md markdown.Markdown[markdown.RuleFrontMatter] + reason string // selector match explanation + tokens int +} + // Run executes the context assembly for the given taskName and returns the assembled result. // The taskName is looked up in task search paths and its content is parsed into blocks. // If the taskName cannot be found as a task file, an error is returned. @@ -803,8 +825,12 @@ func (cc *Context) cleanupDownloadedDirectories() { } } +// findExecuteRuleFiles discovers rule files in each rule directory, then +// bootstraps and publishes them. Discovery of one directory finishes before +// any of that directory's bootstrap scripts run. A lenient bootstrap failure +// skips only that rule; a parse or expand error still stops discovery of the +// rest of that directory. func (cc *Context) findExecuteRuleFiles(ctx context.Context) error { - // Skip rule file discovery if bootstrap is disabled if !cc.doBootstrap { return nil } @@ -813,51 +839,107 @@ func (cc *Context) findExecuteRuleFiles(ctx context.Context) error { return namespacedRuleSearchPaths(dir, cc.namespace) } - err := cc.visitMarkdownFiles(namespacedRulePaths, func(path string, baseFm *markdown.BaseFrontMatter) error { - var frontmatter markdown.RuleFrontMatter + for _, sp := range cc.downloadedPaths { + for _, dir := range namespacedRulePaths(sp.Path) { + var pending []pendingRule - md, err := markdown.ParseMarkdownFileWithLogger(path, &frontmatter, cc.logger) - if err != nil { - return fmt.Errorf("failed to parse markdown file %s: %w", path, err) - } + discoveryErr := cc.visitMarkdownInDir(dir, func(path string, baseFm *markdown.BaseFrontMatter) error { + rule, err := cc.discoverPendingRule(path, baseFm) + if err != nil { + return err + } - if cc.lintCollector != nil { - cc.lintCollector.recordFile(path, LoadedFileKindRule) - } + pending = append(pending, rule) - if frontmatter.Name == "" { - frontmatter.Name = nameFromPath(path) - } + return nil + }) + if discoveryErr != nil { + if !sp.Lenient { + return discoveryErr + } - // Expand parameters only if expand is not explicitly set to false - var processedContent string - if shouldExpandParams(frontmatter.ExpandParams) { - processedContent, err = cc.expandParams(md.Content, nil) - if err != nil { - return fmt.Errorf("failed to expand parameters in file %s: %w", path, err) + cc.logger.Warn("stopping rule discovery after error", "path", dir, "error", discoveryErr) + } + + if err := cc.bootstrapAndPublishRules(ctx, pending, sp.Lenient); err != nil { + return err } - } else { - processedContent = md.Content } + } - tokens := tokencount.EstimateTokens(processedContent) + return nil +} + +// discoverPendingRule parses and expands a rule file without running its bootstrap script. +func (cc *Context) discoverPendingRule( + path string, + baseFm *markdown.BaseFrontMatter, +) (pendingRule, error) { + var frontmatter markdown.RuleFrontMatter - cc.rules = append(cc.rules, markdown.FromContent(frontmatter, processedContent)) + md, err := markdown.ParseMarkdownFileWithLogger(path, &frontmatter, cc.logger) + if err != nil { + return pendingRule{}, &ruleFileError{path: path, err: fmt.Errorf("parse markdown file: %w", err)} + } - cc.totalTokens += tokens + if cc.lintCollector != nil { + cc.lintCollector.recordFile(path, LoadedFileKindRule) + } - // Get match reason to explain why this rule was included - _, reason := cc.includes.MatchesIncludes(*baseFm, cc.includeByDefault) - cc.logger.Info("Including rule file", "path", path, "reason", reason, "tokens", tokens) + if frontmatter.Name == "" { + frontmatter.Name = nameFromPath(path) + } - if err := cc.runBootstrapScript(ctx, path, frontmatter.Bootstrap); err != nil { - return fmt.Errorf("failed to run bootstrap script: %w", err) + processedContent := md.Content + if shouldExpandParams(frontmatter.ExpandParams) { + processedContent, err = cc.expandParams(md.Content, nil) + if err != nil { + return pendingRule{}, &ruleFileError{path: path, err: fmt.Errorf("expand parameters: %w", err)} } + } - return nil - }) - if err != nil { - return fmt.Errorf("failed to find and execute rule files: %w", err) + tokens := tokencount.EstimateTokens(processedContent) + _, reason := cc.includes.MatchesIncludes(*baseFm, cc.includeByDefault) + + return pendingRule{ + path: path, + md: markdown.FromContent(frontmatter, processedContent), + reason: reason, + tokens: tokens, + }, nil +} + +// bootstrapAndPublishRules runs each pending rule's bootstrap script and, on +// success, publishes the rule. When lenient is true, a bootstrap failure logs +// a warning and continues with the remaining rules. +func (cc *Context) bootstrapAndPublishRules( + ctx context.Context, + pending []pendingRule, + lenient bool, +) error { + for _, rule := range pending { + if err := cc.runBootstrapScript(ctx, rule.path, rule.md.FrontMatter.Bootstrap); err != nil { + if lenient { + cc.logger.Warn( + "skipping rule file after bootstrap failure", + "path", rule.path, + "error", err, + ) + + continue + } + + return &ruleFileError{path: rule.path, err: fmt.Errorf("run bootstrap script: %w", err)} + } + + cc.rules = append(cc.rules, rule.md) + cc.totalTokens += rule.tokens + cc.logger.Info( + "Including rule file", + "path", rule.path, + "reason", rule.reason, + "tokens", rule.tokens, + ) } return nil diff --git a/pkg/codingcontext/context_bootstrap_isolation_test.go b/pkg/codingcontext/context_bootstrap_isolation_test.go new file mode 100644 index 0000000..184f257 --- /dev/null +++ b/pkg/codingcontext/context_bootstrap_isolation_test.go @@ -0,0 +1,318 @@ +package codingcontext + +import ( + "context" + "errors" + "fmt" + "log/slog" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/kitproj/coding-context-cli/pkg/codingcontext/markdown" + "github.com/kitproj/coding-context-cli/pkg/codingcontext/taskparser" + "github.com/kitproj/coding-context-cli/pkg/codingcontext/tokencount" +) + +func bootstrapIsolationRuleNames(rules []markdown.Markdown[markdown.RuleFrontMatter]) []string { + names := make([]string, 0, len(rules)) + for _, rule := range rules { + names = append(names, rule.FrontMatter.Name) + } + + return names +} + +func bootstrapIsolationRuleContent( + tb testing.TB, + rules []markdown.Markdown[markdown.RuleFrontMatter], + name string, +) string { + tb.Helper() + + for _, rule := range rules { + if rule.FrontMatter.Name == name { + return rule.Content + } + } + + tb.Fatalf("rule %q not found", name) + + return "" +} + +func rewriteBootstrapIsolationRule(tb testing.TB, path, name, content string) { + tb.Helper() + + source := fmt.Sprintf("---\nname: %s\n---\n%s", name, content) + require.NoError(tb, os.WriteFile(path, []byte(source), 0o600)) +} + +func requireRuleFileError(tb testing.TB, err error, name string) { + tb.Helper() + + var rfe *ruleFileError + require.ErrorAs(tb, err, &rfe) + require.Equal(tb, name, filepath.Base(rfe.path)) +} + +func TestRun_LenientBootstrapFailureSkipsOnlyFailingRule(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + createTask(t, dir, "test-task", "", "Task content") + createRule(t, dir, ".agents/rules/01-before.md", "name: before", "Before content") + createRule(t, dir, ".agents/rules/02-failing.md", "name: failing", "Failing content") + createRule(t, dir, ".agents/rules/03-after.md", "name: after", "After content") + createBootstrapScript(t, dir, ".agents/rules/01-before.md", "#!/bin/sh\nexit 0") + createBootstrapScript(t, dir, ".agents/rules/02-failing.md", "#!/bin/sh\nexit 1") + createBootstrapScript(t, dir, ".agents/rules/03-after.md", "#!/bin/sh\nexit 0") + + bootstrapErr := errors.New("bootstrap sentinel") + var calls []string + var logs strings.Builder + logger := slog.New(slog.NewTextHandler(&logs, nil)) + + cc := New( + WithLenientSearchPaths("file://"+dir), + WithLogger(logger), + ) + cc.cmdRunner = func(cmd *exec.Cmd) error { + name := filepath.Base(cmd.Path) + calls = append(calls, name) + if name == "02-failing-bootstrap" { + return bootstrapErr + } + return nil + } + + result, err := cc.Run(context.Background(), "test-task") + require.NoError(t, err) + require.Equal(t, []string{"before", "after"}, bootstrapIsolationRuleNames(result.Rules)) + require.Equal(t, []string{ + "01-before-bootstrap", + "02-failing-bootstrap", + "03-after-bootstrap", + }, calls) + require.Contains(t, result.Prompt, "Before content") + require.NotContains(t, result.Prompt, "Failing content") + require.Contains(t, result.Prompt, "After content") + require.Contains(t, logs.String(), "skipping rule file after bootstrap failure") + require.Contains(t, logs.String(), "02-failing.md") + require.Contains(t, logs.String(), bootstrapErr.Error()) + + expectedTokens := result.Task.Tokens + for _, rule := range result.Rules { + expectedTokens += tokencount.EstimateTokens(rule.Content) + } + require.Equal(t, expectedTokens, result.Tokens) +} + +func TestRun_StrictBootstrapFailureIsFatal(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + createTask(t, dir, "test-task", "", "Task content") + createRule(t, dir, ".agents/rules/01-before.md", "name: before", "Before content") + createRule(t, dir, ".agents/rules/02-failing.md", "name: failing", "Failing content") + createRule(t, dir, ".agents/rules/03-after.md", "name: after", "After content") + createBootstrapScript(t, dir, ".agents/rules/01-before.md", "#!/bin/sh\nexit 0") + createBootstrapScript(t, dir, ".agents/rules/02-failing.md", "#!/bin/sh\nexit 1") + createBootstrapScript(t, dir, ".agents/rules/03-after.md", "#!/bin/sh\nexit 0") + + bootstrapErr := errors.New("bootstrap sentinel") + var calls []string + + cc := New(WithSearchPaths("file://" + dir)) + cc.cmdRunner = func(cmd *exec.Cmd) error { + name := filepath.Base(cmd.Path) + calls = append(calls, name) + if name == "02-failing-bootstrap" { + return bootstrapErr + } + return nil + } + + result, err := cc.Run(context.Background(), "test-task") + require.Nil(t, result) + require.ErrorIs(t, err, bootstrapErr) + requireRuleFileError(t, err, "02-failing.md") + require.Equal(t, []string{ + "01-before-bootstrap", + "02-failing-bootstrap", + }, calls) + require.Equal(t, []string{"before"}, bootstrapIsolationRuleNames(cc.rules)) +} + +func TestRun_RuleDirectoryDiscoveryCompletesBeforeBootstrap(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + targetPath := filepath.Join(dir, ".agents", "rules", "02-target.md") + + createTask(t, dir, "test-task", "", "Task content") + createRule(t, dir, ".agents/rules/01-writer.md", "name: writer", "Writer content") + createRule(t, dir, ".agents/rules/02-target.md", "name: target", "Original target content") + createBootstrapScript(t, dir, ".agents/rules/01-writer.md", "#!/bin/sh\nexit 0") + + cc := New(WithSearchPaths("file://" + dir)) + cc.cmdRunner = func(_ *exec.Cmd) error { + rewriteBootstrapIsolationRule(t, targetPath, "target", "Rewritten target content") + return nil + } + + result, err := cc.Run(context.Background(), "test-task") + require.NoError(t, err) + require.Contains(t, bootstrapIsolationRuleContent(t, result.Rules, "target"), "Original target content") + require.NotContains(t, bootstrapIsolationRuleContent(t, result.Rules, "target"), "Rewritten target content") +} + +func TestRun_BootstrapCompletesBeforeNextRuleDirectoryDiscovery(t *testing.T) { + t.Parallel() + + firstDir := t.TempDir() + secondDir := t.TempDir() + targetPath := filepath.Join(secondDir, ".agents", "rules", "01-target.md") + + createTask(t, firstDir, "test-task", "", "Task content") + createRule(t, firstDir, ".agents/rules/01-writer.md", "name: writer", "Writer content") + createBootstrapScript(t, firstDir, ".agents/rules/01-writer.md", "#!/bin/sh\nexit 0") + createRule(t, secondDir, ".agents/rules/01-target.md", "name: target", "Original target content") + + cc := New( + WithSearchPaths("file://"+firstDir), + WithSearchPaths("file://"+secondDir), + ) + cc.cmdRunner = func(_ *exec.Cmd) error { + rewriteBootstrapIsolationRule(t, targetPath, "target", "Rewritten before later discovery") + return nil + } + + result, err := cc.Run(context.Background(), "test-task") + require.NoError(t, err) + require.Contains(t, bootstrapIsolationRuleContent(t, result.Rules, "target"), "Rewritten before later discovery") +} + +func TestRun_LenientRuleParseFailurePreservesEarlierRules(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + createTask(t, dir, "test-task", "", "Task content") + createRule(t, dir, ".agents/rules/01-before.md", "name: before", "Before content") + createBootstrapScript(t, dir, ".agents/rules/01-before.md", "#!/bin/sh\nexit 0") + createRule(t, dir, ".agents/rules/02-failing.md", "name: failing\nexpand:\n - true", "Failing content") + createRule(t, dir, ".agents/rules/03-after.md", "name: after", "After content") + + var calls []string + var logs strings.Builder + logger := slog.New(slog.NewTextHandler(&logs, nil)) + + cc := New( + WithLenientSearchPaths("file://"+dir), + WithLogger(logger), + ) + cc.cmdRunner = func(cmd *exec.Cmd) error { + calls = append(calls, filepath.Base(cmd.Path)) + return nil + } + + result, err := cc.Run(context.Background(), "test-task") + require.NoError(t, err) + require.Equal(t, []string{"before"}, bootstrapIsolationRuleNames(result.Rules)) + require.Equal(t, []string{"01-before-bootstrap"}, calls) + require.Contains(t, logs.String(), "stopping rule discovery after error") + require.Contains(t, logs.String(), "02-failing.md") + require.NotContains(t, result.Prompt, "After content") +} + +func TestRun_LenientRuleParseFailureDoesNotBlockLaterLenientSearchPath(t *testing.T) { + t.Parallel() + + firstDir := t.TempDir() + secondDir := t.TempDir() + + createTask(t, firstDir, "test-task", "", "Task content") + createRule(t, firstDir, ".agents/rules/01-before.md", "name: before", "Before content") + createBootstrapScript(t, firstDir, ".agents/rules/01-before.md", "#!/bin/sh\nexit 0") + createRule(t, firstDir, ".agents/rules/02-failing.md", "name: failing\nexpand:\n - true", "Failing content") + createRule(t, firstDir, ".agents/rules/03-after.md", "name: after", "After content") + createRule(t, secondDir, ".agents/rules/01-target.md", "name: target", "Target content") + createBootstrapScript(t, secondDir, ".agents/rules/01-target.md", "#!/bin/sh\nexit 0") + + var calls []string + var logs strings.Builder + logger := slog.New(slog.NewTextHandler(&logs, nil)) + + cc := New( + WithLenientSearchPaths("file://"+firstDir), + WithLenientSearchPaths("file://"+secondDir), + WithLogger(logger), + ) + cc.cmdRunner = func(cmd *exec.Cmd) error { + calls = append(calls, filepath.Base(cmd.Path)) + return nil + } + + result, err := cc.Run(context.Background(), "test-task") + require.NoError(t, err) + require.Equal(t, []string{"before", "target"}, bootstrapIsolationRuleNames(result.Rules)) + require.Equal(t, []string{"01-before-bootstrap", "01-target-bootstrap"}, calls) + require.Contains(t, result.Prompt, "Before content") + require.NotContains(t, result.Prompt, "Failing content") + require.NotContains(t, result.Prompt, "After content") + require.Contains(t, result.Prompt, "Target content") + require.Contains(t, logs.String(), "stopping rule discovery after error") + require.Contains(t, logs.String(), "02-failing.md") +} + +func TestRun_StrictRuleParseFailureIsFatalBeforeDirectoryBootstrap(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + createTask(t, dir, "test-task", "", "Task content") + createRule(t, dir, ".agents/rules/01-before.md", "name: before", "Before content") + createBootstrapScript(t, dir, ".agents/rules/01-before.md", "#!/bin/sh\nexit 0") + createRule(t, dir, ".agents/rules/02-failing.md", "name: failing\nexpand:\n - true", "Failing content") + + var calls []string + cc := New(WithSearchPaths("file://" + dir)) + cc.cmdRunner = func(cmd *exec.Cmd) error { + calls = append(calls, filepath.Base(cmd.Path)) + return nil + } + + result, err := cc.Run(context.Background(), "test-task") + require.Nil(t, result) + require.Error(t, err) + requireRuleFileError(t, err, "02-failing.md") + require.Empty(t, calls) + require.Empty(t, cc.rules) +} + +func TestRun_RuleExpansionUsesDirectoryDiscoverySnapshot(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + createTask(t, dir, "test-task", "", "Task content") + createRule(t, dir, ".agents/rules/01-writer.md", "name: writer", "Writer content") + createBootstrapScript(t, dir, ".agents/rules/01-writer.md", "#!/bin/sh\nexit 0") + createRule(t, dir, ".agents/rules/02-target.md", "name: target\nexpand: true", "Value: ${value}") + + cc := New( + WithSearchPaths("file://"+dir), + WithParams(taskparser.Params{"value": []string{"before-bootstrap"}}), + ) + cc.cmdRunner = func(_ *exec.Cmd) error { + cc.params = taskparser.Params{"value": []string{"after-bootstrap"}} + return nil + } + + result, err := cc.Run(context.Background(), "test-task") + require.NoError(t, err) + require.Contains(t, bootstrapIsolationRuleContent(t, result.Rules, "target"), "before-bootstrap") + require.NotContains(t, bootstrapIsolationRuleContent(t, result.Rules, "target"), "after-bootstrap") +} diff --git a/pkg/codingcontext/context_edge_cases_test.go b/pkg/codingcontext/context_edge_cases_test.go index 55cb09d..5a73d81 100644 --- a/pkg/codingcontext/context_edge_cases_test.go +++ b/pkg/codingcontext/context_edge_cases_test.go @@ -147,9 +147,8 @@ func TestSkillValidation_DescOverLimit(t *testing.T) { } // TestBootstrapFailurePropagates verifies that when the bootstrap script runner -// returns an error, Run() propagates it as a failure. The rule is appended to -// cc.rules before bootstrap runs, so the error must surface via Run() rather than -// being silently ignored. +// returns an error, Run propagates it as a failure. The rule is not published +// unless bootstrap succeeds. func TestBootstrapFailurePropagates(t *testing.T) { t.Parallel() @@ -159,18 +158,23 @@ func TestBootstrapFailurePropagates(t *testing.T) { createBootstrapScript(t, dir, ".agents/rules/rule.md", "#!/bin/sh\nexit 1") c := New(WithSearchPaths(dir)) - // Replace the default runner with one that always fails + bootstrapErr := errors.New("simulated bootstrap failure") //nolint:err113 c.cmdRunner = func(_ *exec.Cmd) error { - return errors.New("simulated bootstrap failure") //nolint:err113 + return bootstrapErr } _, err := c.Run(context.Background(), "task") - if err == nil { - t.Fatal("expected Run() to fail when bootstrap script errors") + if !errors.Is(err, bootstrapErr) { + t.Fatalf("Run() error = %v, want %v", err, bootstrapErr) + } + + var rfe *ruleFileError + if !errors.As(err, &rfe) { + t.Fatalf("Run() error type = %T, want *ruleFileError", err) } - if !strings.Contains(err.Error(), "bootstrap") { - t.Errorf("expected bootstrap-related error message, got: %v", err) + if filepath.Base(rfe.path) != "rule.md" { + t.Errorf("rule path = %q, want rule.md", rfe.path) } }