From 2450e814f9ace6eaa7196c6adad0030cd086db89 Mon Sep 17 00:00:00 2001 From: r4ravi2008 Date: Tue, 22 Sep 2026 17:10:22 -0700 Subject: [PATCH 1/3] fix: isolate lenient rule bootstrap failures Bootstrap ran inside the directory walk, so one failed script aborted the rest of that directory. Discover and expand a directory first, then run its bootstraps. On a lenient path, skip only the failing rule. --- pkg/codingcontext/context.go | 125 +++++-- .../context_bootstrap_isolation_test.go | 312 ++++++++++++++++++ 2 files changed, 404 insertions(+), 33 deletions(-) create mode 100644 pkg/codingcontext/context_bootstrap_isolation_test.go diff --git a/pkg/codingcontext/context.go b/pkg/codingcontext/context.go index cd4b198..8881eb4 100644 --- a/pkg/codingcontext/context.go +++ b/pkg/codingcontext/context.go @@ -144,6 +144,13 @@ func nameFromPath(path string) string { type markdownVisitor func(path string, fm *markdown.BaseFrontMatter) error +type pendingRule struct { + path string + rule markdown.Markdown[markdown.RuleFrontMatter] + reason string + 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. @@ -804,7 +811,6 @@ func (cc *Context) cleanupDownloadedDirectories() { } func (cc *Context) findExecuteRuleFiles(ctx context.Context) error { - // Skip rule file discovery if bootstrap is disabled if !cc.doBootstrap { return nil } @@ -813,51 +819,104 @@ 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) + return nil + }) - if frontmatter.Name == "" { - frontmatter.Name = nameFromPath(path) - } + if discoveryErr != nil && !sp.Lenient { + return fmt.Errorf("failed to find and execute rule files: %w", 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) + if err := cc.bootstrapAndPublishRules(ctx, pending, sp.Lenient); err != nil { + return fmt.Errorf("failed to find and execute rule files: %w", err) + } + + if discoveryErr != nil { + cc.logger.Warn("skipping directory", "path", dir, "error", discoveryErr) } - } else { - processedContent = md.Content } + } + + return nil +} - tokens := tokencount.EstimateTokens(processedContent) +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{}, fmt.Errorf("failed to parse markdown file %s: %w", path, 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) + var processedContent string + if shouldExpandParams(frontmatter.ExpandParams) { + processedContent, err = cc.expandParams(md.Content, nil) + if err != nil { + return pendingRule{}, fmt.Errorf("failed to expand parameters in file %s: %w", path, err) } + } else { + processedContent = md.Content + } - 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, + rule: markdown.FromContent(frontmatter, processedContent), + reason: reason, + tokens: tokens, + }, nil +} + +func (cc *Context) bootstrapAndPublishRules( + ctx context.Context, + pending []pendingRule, + lenient bool, +) error { + for _, rule := range pending { + if err := cc.runBootstrapScript(ctx, rule.path, rule.rule.FrontMatter.Bootstrap); err != nil { + if lenient { + cc.logger.Warn( + "skipping rule file after bootstrap failure", + "path", rule.path, + "error", err, + ) + continue + } + + return fmt.Errorf("failed to run bootstrap script for rule %s: %w", rule.path, err) + } + + cc.rules = append(cc.rules, rule.rule) + 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..c67378b --- /dev/null +++ b/pkg/codingcontext/context_bootstrap_isolation_test.go @@ -0,0 +1,312 @@ +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( + t *testing.T, + rules []markdown.Markdown[markdown.RuleFrontMatter], + name string, +) string { + t.Helper() + + for _, rule := range rules { + if rule.FrontMatter.Name == name { + return rule.Content + } + } + + t.Fatalf("rule %q not found", name) + return "" +} + +func rewriteBootstrapIsolationRule(t *testing.T, path, name, content string) { + t.Helper() + + source := fmt.Sprintf("---\nname: %s\n---\n%s", name, content) + require.NoError(t, os.WriteFile(path, []byte(source), 0o600)) +} + +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) + hypotheticalWithFailing := expectedTokens + tokencount.EstimateTokens("Failing content") + if tokencount.EstimateTokens("Failing content") > 0 { + require.NotEqual(t, hypotheticalWithFailing, 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) + require.Contains(t, err.Error(), "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(), "skipping directory") + 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(), "skipping directory") + 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) + require.Contains(t, err.Error(), "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") +} From f9319b085048117566f545dbbd17ff911788acb6 Mon Sep 17 00:00:00 2001 From: r4ravi2008 Date: Tue, 22 Sep 2026 17:10:22 -0700 Subject: [PATCH 2/3] docs: document rule bootstrap failure handling Describe the discover-then-bootstrap order and how strict vs lenient search paths treat bootstrap and parse failures. --- docs/reference/search-paths.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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: From 2f88b4ce1a6bdf6e69bfe3d6f0a877946da2153b Mon Sep 17 00:00:00 2001 From: r4ravi2008 Date: Tue, 22 Sep 2026 22:06:17 -0700 Subject: [PATCH 3/3] fix: carry the failing rule path on bootstrap errors Stop wrapping the same find-and-execute message twice so callers can match the rule file without reading error text. --- pkg/codingcontext/context.go | 69 ++++++++++++------- .../context_bootstrap_isolation_test.go | 34 +++++---- pkg/codingcontext/context_edge_cases_test.go | 22 +++--- 3 files changed, 79 insertions(+), 46 deletions(-) diff --git a/pkg/codingcontext/context.go b/pkg/codingcontext/context.go index 8881eb4..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,10 +144,25 @@ 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 - rule markdown.Markdown[markdown.RuleFrontMatter] - reason string + md markdown.Markdown[markdown.RuleFrontMatter] + reason string // selector match explanation tokens int } @@ -810,6 +825,11 @@ 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 { if !cc.doBootstrap { return nil @@ -830,19 +850,19 @@ func (cc *Context) findExecuteRuleFiles(ctx context.Context) error { } pending = append(pending, rule) + return nil }) + if discoveryErr != nil { + if !sp.Lenient { + return discoveryErr + } - if discoveryErr != nil && !sp.Lenient { - return fmt.Errorf("failed to find and execute rule files: %w", discoveryErr) + cc.logger.Warn("stopping rule discovery after error", "path", dir, "error", discoveryErr) } if err := cc.bootstrapAndPublishRules(ctx, pending, sp.Lenient); err != nil { - return fmt.Errorf("failed to find and execute rule files: %w", err) - } - - if discoveryErr != nil { - cc.logger.Warn("skipping directory", "path", dir, "error", discoveryErr) + return err } } } @@ -850,6 +870,7 @@ func (cc *Context) findExecuteRuleFiles(ctx context.Context) error { return nil } +// discoverPendingRule parses and expands a rule file without running its bootstrap script. func (cc *Context) discoverPendingRule( path string, baseFm *markdown.BaseFrontMatter, @@ -858,7 +879,7 @@ func (cc *Context) discoverPendingRule( md, err := markdown.ParseMarkdownFileWithLogger(path, &frontmatter, cc.logger) if err != nil { - return pendingRule{}, fmt.Errorf("failed to parse markdown file %s: %w", path, err) + return pendingRule{}, &ruleFileError{path: path, err: fmt.Errorf("parse markdown file: %w", err)} } if cc.lintCollector != nil { @@ -869,14 +890,12 @@ func (cc *Context) discoverPendingRule( frontmatter.Name = nameFromPath(path) } - var processedContent string + processedContent := md.Content if shouldExpandParams(frontmatter.ExpandParams) { processedContent, err = cc.expandParams(md.Content, nil) if err != nil { - return pendingRule{}, fmt.Errorf("failed to expand parameters in file %s: %w", path, err) + return pendingRule{}, &ruleFileError{path: path, err: fmt.Errorf("expand parameters: %w", err)} } - } else { - processedContent = md.Content } tokens := tokencount.EstimateTokens(processedContent) @@ -884,32 +903,36 @@ func (cc *Context) discoverPendingRule( return pendingRule{ path: path, - rule: markdown.FromContent(frontmatter, processedContent), + 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.rule.FrontMatter.Bootstrap); err != nil { + 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 fmt.Errorf("failed to run bootstrap script for rule %s: %w", rule.path, err) + return &ruleFileError{path: rule.path, err: fmt.Errorf("run bootstrap script: %w", err)} } - cc.rules = append(cc.rules, rule.rule) + cc.rules = append(cc.rules, rule.md) cc.totalTokens += rule.tokens cc.logger.Info( "Including rule file", diff --git a/pkg/codingcontext/context_bootstrap_isolation_test.go b/pkg/codingcontext/context_bootstrap_isolation_test.go index c67378b..184f257 100644 --- a/pkg/codingcontext/context_bootstrap_isolation_test.go +++ b/pkg/codingcontext/context_bootstrap_isolation_test.go @@ -23,15 +23,16 @@ func bootstrapIsolationRuleNames(rules []markdown.Markdown[markdown.RuleFrontMat for _, rule := range rules { names = append(names, rule.FrontMatter.Name) } + return names } func bootstrapIsolationRuleContent( - t *testing.T, + tb testing.TB, rules []markdown.Markdown[markdown.RuleFrontMatter], name string, ) string { - t.Helper() + tb.Helper() for _, rule := range rules { if rule.FrontMatter.Name == name { @@ -39,15 +40,24 @@ func bootstrapIsolationRuleContent( } } - t.Fatalf("rule %q not found", name) + tb.Fatalf("rule %q not found", name) + return "" } -func rewriteBootstrapIsolationRule(t *testing.T, path, name, content string) { - t.Helper() +func rewriteBootstrapIsolationRule(tb testing.TB, path, name, content string) { + tb.Helper() source := fmt.Sprintf("---\nname: %s\n---\n%s", name, content) - require.NoError(t, os.WriteFile(path, []byte(source), 0o600)) + 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) { @@ -100,10 +110,6 @@ func TestRun_LenientBootstrapFailureSkipsOnlyFailingRule(t *testing.T) { expectedTokens += tokencount.EstimateTokens(rule.Content) } require.Equal(t, expectedTokens, result.Tokens) - hypotheticalWithFailing := expectedTokens + tokencount.EstimateTokens("Failing content") - if tokencount.EstimateTokens("Failing content") > 0 { - require.NotEqual(t, hypotheticalWithFailing, result.Tokens) - } } func TestRun_StrictBootstrapFailureIsFatal(t *testing.T) { @@ -134,7 +140,7 @@ func TestRun_StrictBootstrapFailureIsFatal(t *testing.T) { result, err := cc.Run(context.Background(), "test-task") require.Nil(t, result) require.ErrorIs(t, err, bootstrapErr) - require.Contains(t, err.Error(), "02-failing.md") + requireRuleFileError(t, err, "02-failing.md") require.Equal(t, []string{ "01-before-bootstrap", "02-failing-bootstrap", @@ -218,7 +224,7 @@ func TestRun_LenientRuleParseFailurePreservesEarlierRules(t *testing.T) { 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(), "skipping directory") + 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") } @@ -259,7 +265,7 @@ func TestRun_LenientRuleParseFailureDoesNotBlockLaterLenientSearchPath(t *testin 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(), "skipping directory") + require.Contains(t, logs.String(), "stopping rule discovery after error") require.Contains(t, logs.String(), "02-failing.md") } @@ -282,7 +288,7 @@ func TestRun_StrictRuleParseFailureIsFatalBeforeDirectoryBootstrap(t *testing.T) result, err := cc.Run(context.Background(), "test-task") require.Nil(t, result) require.Error(t, err) - require.Contains(t, err.Error(), "02-failing.md") + requireRuleFileError(t, err, "02-failing.md") require.Empty(t, calls) require.Empty(t, cc.rules) } 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) } }