From 08452d0fe885bf2aa93a4499e96c314eba859523 Mon Sep 17 00:00:00 2001 From: Corey Ryan Dean Date: Sun, 17 May 2026 09:06:56 -0500 Subject: [PATCH] fix(intent): ground explicit file edits before proposing them --- docs/SPEC.md | 7 ++ internal/engine/engine.go | 21 ++++ internal/engine/engine_test.go | 146 ++++++++++++++++++++++++++ internal/engine/file_grounding.go | 165 ++++++++++++++++++++++++++++++ internal/model/prompt.go | 10 +- 5 files changed, 348 insertions(+), 1 deletion(-) create mode 100644 internal/engine/file_grounding.go diff --git a/docs/SPEC.md b/docs/SPEC.md index cb6f800..f916765 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -219,6 +219,13 @@ Tool calls outside this catalog are rejected and fed back to the model as an err If the model exceeds `max_tool_steps`, intent forces a `refuse` with reason `"exceeded tool-call budget"` and does not execute anything. +For explicit file-edit requests, the multi-step loop must ground the edit in +the target file's real contents before it emits a mutating command or script. +If the user asks to edit, update, rewrite, replace, append to, or remove from +a specific file path, the model must `read_file` that file first (resolving +`~` or env-dependent paths as needed). Emitting a shell snippet that does not +touch the named file is a contract bug, not an acceptable approximation. + ### 2.5 System prompt structure The system prompt is assembled by intent at runtime and includes: diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 50dd5b5..a7f99ad 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -115,6 +115,7 @@ func (e *Engine) Run(ctx context.Context, prompt string, opts Options) (*Result, {Role: "system", Content: sysPrompt}, {Role: "user", Content: prompt}, } + toolCalls := make([]toolCallRecord, 0, opts.MaxToolSteps) if opts.OnPhase != nil { opts.OnPhase("Understanding...") @@ -136,6 +137,22 @@ func (e *Engine) Run(ctx context.Context, prompt string, opts Options) (*Result, } if resp.Approach != model.ApproachToolCall { + if missingTargets, needsRepair := needsFileGrounding(prompt, resp, toolCalls, pack.Cwd); needsRepair { + if step == opts.MaxToolSteps { + res.Response = buildFileGroundingRefusal(missingTargets) + res.ToolStepsUsed = step + return res, nil + } + msgs = append(msgs, model.Message{ + Role: "assistant", + Content: jsonMust(resp), + }) + msgs = append(msgs, model.Message{ + Role: "system", + Content: buildFileGroundingRepairMessage(missingTargets), + }) + continue + } gr := safety.Apply(resp) res.Response = resp res.GuardResult = gr @@ -171,6 +188,10 @@ func (e *Engine) Run(ctx context.Context, prompt string, opts Options) (*Result, vl.Section(fmt.Sprintf("tool call (step %d)", step+1)) vl.KV("name", resp.ToolCall.Name) vl.RawBytes("arguments", resp.ToolCall.Arguments) + toolCalls = append(toolCalls, toolCallRecord{ + Name: resp.ToolCall.Name, + Arguments: resp.ToolCall.Arguments, + }) out, err := tools.Run(ctx, opts.ToolHost, resp.ToolCall.Name, resp.ToolCall.Arguments) if err != nil { out = tools.Result{"error": err.Error()} diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go index f332eca..5249978 100644 --- a/internal/engine/engine_test.go +++ b/internal/engine/engine_test.go @@ -2,6 +2,10 @@ package engine import ( "context" + "encoding/json" + "fmt" + "os" + "path/filepath" "strings" "testing" @@ -17,6 +21,15 @@ type captureSystemPromptBackend struct { systemPrompt string } +type fileGroundingBackend struct { + targetPath string + calls int +} + +type stubbornFileGroundingBackend struct { + calls int +} + func (b *captureSystemPromptBackend) Name() string { return "capture" } func (b *captureSystemPromptBackend) Available(context.Context) error { return nil } @@ -36,6 +49,71 @@ func (b *captureSystemPromptBackend) Complete(_ context.Context, req model.Compl }, nil } +func (b *fileGroundingBackend) Name() string { return "file-grounding" } + +func (b *fileGroundingBackend) Available(context.Context) error { return nil } + +func (b *fileGroundingBackend) Complete(_ context.Context, req model.CompleteRequest) (*model.Response, error) { + b.calls++ + last := req.Messages[len(req.Messages)-1] + switch { + case last.Role == "tool" && last.Name == "read_file": + return &model.Response{ + IntentSummary: "Update the requested file in place.", + Approach: model.ApproachScript, + Script: &model.Script{ + Interpreter: "python3", + Body: fmt.Sprintf( + "from pathlib import Path\np = Path(%q)\np.write_text('updated\\n')\n", + b.targetPath, + ), + }, + Description: "Rewrite the target file with the requested content.", + Risk: model.RiskMutates, + ExpectedRuntime: model.RuntimeInstant, + Confidence: model.ConfidenceHigh, + }, nil + case last.Role == "system" && strings.Contains(last.Content, "without first reading the target file"): + args, _ := json.Marshal(map[string]any{"path": b.targetPath}) + return &model.Response{ + IntentSummary: "Read the target file before editing it.", + Approach: model.ApproachToolCall, + ToolCall: &model.ToolCall{ + Name: "read_file", + Arguments: args, + }, + Description: "Read the current file contents first.", + }, nil + default: + return &model.Response{ + IntentSummary: "Update the requested file.", + Approach: model.ApproachScript, + Script: &model.Script{Interpreter: "zsh", Body: "export CLAUDE_CONFIG_DIR=\"$HOME/.claude-test\"\nexec claude \"$@\"\n"}, + Description: "Print a shell snippet for the requested change.", + Risk: model.RiskSafe, + ExpectedRuntime: model.RuntimeInstant, + Confidence: model.ConfidenceMedium, + }, nil + } +} + +func (b *stubbornFileGroundingBackend) Name() string { return "stubborn-file-grounding" } + +func (b *stubbornFileGroundingBackend) Available(context.Context) error { return nil } + +func (b *stubbornFileGroundingBackend) Complete(context.Context, model.CompleteRequest) (*model.Response, error) { + b.calls++ + return &model.Response{ + IntentSummary: "Update the requested file.", + Approach: model.ApproachScript, + Script: &model.Script{Interpreter: "zsh", Body: "echo replace these lines manually"}, + Description: "Print a shell snippet for the requested change.", + Risk: model.RiskSafe, + ExpectedRuntime: model.RuntimeInstant, + Confidence: model.ConfidenceMedium, + }, nil +} + func (b *cacheIdentityBackend) Name() string { return b.name } func (b *cacheIdentityBackend) Available(context.Context) error { return nil } @@ -98,6 +176,23 @@ func TestRunInjectsUserContextIntoSystemPrompt(t *testing.T) { } } +func TestRunSystemPromptRequiresReadFileBeforeEdit(t *testing.T) { + eng := New(nil) + be := &captureSystemPromptBackend{} + _, err := eng.Run(context.Background(), "edit ~/.zshrc to add a new alias", Options{ + Backend: be, + }) + if err != nil { + t.Fatalf("run: %v", err) + } + if !strings.Contains(be.systemPrompt, "modify, edit, update, rewrite, replace, append to, remove") { + t.Fatalf("system prompt missing explicit file-edit grounding rule: %q", be.systemPrompt) + } + if !strings.Contains(be.systemPrompt, "read_file(path) FIRST") { + t.Fatalf("system prompt missing read_file-first instruction: %q", be.systemPrompt) + } +} + func TestRunCacheKeyIncludesUserContext(t *testing.T) { eng := New(nil) ctx := context.Background() @@ -212,3 +307,54 @@ func TestRunCacheKeyTreatsBlankProjectRCAsAbsent(t *testing.T) { t.Fatalf("blank .intentrc content should preserve the baseline cache key: base=%q blank=%q", base.CacheKey, blank.CacheKey) } } + +func TestRunRepairsUngroundedFileEditResponses(t *testing.T) { + tmpDir := t.TempDir() + target := filepath.Join(tmpDir, "notes.txt") + if err := os.WriteFile(target, []byte("before\n"), 0o644); err != nil { + t.Fatalf("write target: %v", err) + } + + eng := New(nil) + be := &fileGroundingBackend{targetPath: target} + res, err := eng.Run(context.Background(), "update "+target+" to say updated", Options{ + Backend: be, + MaxToolSteps: 4, + }) + if err != nil { + t.Fatalf("run: %v", err) + } + if be.calls != 3 { + t.Fatalf("expected grounding retry plus read_file loop, backend calls=%d", be.calls) + } + if res.Response == nil || res.Response.Approach != model.ApproachScript { + t.Fatalf("expected final grounded script response, got %#v", res.Response) + } + if res.Response.Script == nil || !strings.Contains(res.Response.Script.Body, target) { + t.Fatalf("expected final script to target the grounded file, got %#v", res.Response.Script) + } + if res.ToolStepsUsed != 2 { + t.Fatalf("expected one tool step after grounding repair, got %d", res.ToolStepsUsed) + } +} + +func TestRunRefusesUngroundedFileEditsAfterRepairFailure(t *testing.T) { + eng := New(nil) + be := &stubbornFileGroundingBackend{} + res, err := eng.Run(context.Background(), "update ~/.zshrc to add a new alias", Options{ + Backend: be, + MaxToolSteps: 1, + }) + if err != nil { + t.Fatalf("run: %v", err) + } + if be.calls != 2 { + t.Fatalf("expected initial response plus one repair retry, backend calls=%d", be.calls) + } + if res.Response == nil || res.Response.Approach != model.ApproachRefuse { + t.Fatalf("expected refuse response, got %#v", res.Response) + } + if !strings.Contains(res.Response.RefusalReason, ".zshrc") { + t.Fatalf("expected refusal to mention missing grounded file target, got %q", res.Response.RefusalReason) + } +} diff --git a/internal/engine/file_grounding.go b/internal/engine/file_grounding.go new file mode 100644 index 0000000..1dc5727 --- /dev/null +++ b/internal/engine/file_grounding.go @@ -0,0 +1,165 @@ +package engine + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "regexp" + "slices" + "strings" + + "github.com/CoreyRDean/intent/internal/model" +) + +type toolCallRecord struct { + Name string + Arguments json.RawMessage +} + +var ( + fileEditVerbPattern = regexp.MustCompile(`(?i)\b(modify|edit|update|rewrite|replace|append|prepend|insert|remove|delete|change)\b`) + pathTokenPattern = regexp.MustCompile(`(?:^|[\s"'` + "`" + `])((?:~|\.{1,2}|/)[^\s"'` + "`" + `,;:()]+|(?:\.[A-Za-z0-9_-]+)|(?:[A-Za-z0-9._-]+\.[A-Za-z0-9._-]+))`) +) + +func needsFileGrounding(prompt string, resp *model.Response, calls []toolCallRecord, cwd string) ([]string, bool) { + if resp == nil { + return nil, false + } + if resp.Approach != model.ApproachCommand && resp.Approach != model.ApproachScript { + return nil, false + } + targets := explicitEditTargets(prompt) + if len(targets) == 0 { + return nil, false + } + grounded := groundedTargets(calls, targets, cwd) + missing := make([]string, 0, len(targets)) + for _, target := range targets { + if !grounded[target] { + missing = append(missing, target) + } + } + return missing, len(missing) > 0 +} + +func explicitEditTargets(prompt string) []string { + if !fileEditVerbPattern.MatchString(prompt) { + return nil + } + matches := pathTokenPattern.FindAllStringSubmatch(prompt, -1) + if len(matches) == 0 { + return nil + } + seen := map[string]struct{}{} + out := make([]string, 0, len(matches)) + for _, match := range matches { + if len(match) < 2 { + continue + } + token := cleanPathToken(match[1]) + if token == "" || looksLikeNonPathToken(token) { + continue + } + if _, ok := seen[token]; ok { + continue + } + seen[token] = struct{}{} + out = append(out, token) + } + return out +} + +func groundedTargets(calls []toolCallRecord, targets []string, cwd string) map[string]bool { + grounded := make(map[string]bool, len(targets)) + home, _ := os.UserHomeDir() + for _, call := range calls { + if call.Name != "read_file" { + continue + } + var args struct { + Path string `json:"path"` + } + if err := json.Unmarshal(call.Arguments, &args); err != nil || strings.TrimSpace(args.Path) == "" { + continue + } + for _, target := range targets { + if targetMatchesReadPath(target, args.Path, cwd, home) { + grounded[target] = true + } + } + } + return grounded +} + +func targetMatchesReadPath(target string, readPath string, cwd string, home string) bool { + target = cleanPathToken(target) + readPath = cleanPathToken(readPath) + if target == "" || readPath == "" { + return false + } + normTarget := normalizePathForMatch(target, cwd, home) + normRead := normalizePathForMatch(readPath, cwd, home) + if normTarget != "" && normRead != "" && normTarget == normRead { + return true + } + // Bare filenames and dotfiles are ambiguous about cwd vs home, so fall + // back to basename equality when the user named a single file token. + if !strings.Contains(target, "/") { + return filepath.Base(readPath) == target + } + return false +} + +func normalizePathForMatch(p string, cwd string, home string) string { + switch { + case p == "": + return "" + case strings.HasPrefix(p, "~/"): + if home == "" { + return filepath.Clean(p) + } + return filepath.Clean(filepath.Join(home, strings.TrimPrefix(p, "~/"))) + case filepath.IsAbs(p): + return filepath.Clean(p) + case strings.HasPrefix(p, "./"), strings.HasPrefix(p, "../"): + if cwd == "" { + return filepath.Clean(p) + } + return filepath.Clean(filepath.Join(cwd, p)) + default: + return filepath.Clean(p) + } +} + +func buildFileGroundingRepairMessage(targets []string) string { + list := strings.Join(targets, ", ") + return fmt.Sprintf( + "You returned a command or script for a file-edit request without first reading the target file(s): %s. This is invalid. Your next response MUST be approach=tool_call with read_file on each missing target (resolve ~ or env vars first if needed), or approach=clarify/refuse if the file cannot be read. Do not emit pseudocode or a non-mutating snippet.", + list, + ) +} + +func buildFileGroundingRefusal(targets []string) *model.Response { + list := strings.Join(targets, ", ") + return &model.Response{ + IntentSummary: "Refused: file edit was not grounded in the target file contents.", + Approach: model.ApproachRefuse, + RefusalReason: fmt.Sprintf("intent could not ground the requested edit in the current contents of %s; rerun with a readable path or narrower request", list), + } +} + +func cleanPathToken(token string) string { + token = strings.TrimSpace(token) + token = strings.Trim(token, `"'`+"`") + token = strings.TrimRight(token, ".,;:!?)]}") + token = strings.TrimLeft(token, "([{") + return token +} + +func looksLikeNonPathToken(token string) bool { + if token == "" || strings.HasPrefix(token, "--") || strings.Contains(token, "://") { + return true + } + return slices.Contains([]string{"safe", "network", "mutates", "destructive", "sudo"}, strings.ToLower(token)) +} diff --git a/internal/model/prompt.go b/internal/model/prompt.go index d0fe7d1..0267620 100644 --- a/internal/model/prompt.go +++ b/internal/model/prompt.go @@ -7,7 +7,7 @@ import ( // PromptTemplateVersion is bumped on every prompt change. Skill cache keys // include this; bumping it invalidates the entire cache. -const PromptTemplateVersion = "8" +const PromptTemplateVersion = "9" // SystemPromptInputs is everything intent injects into the system prompt // at request time. Stable across daemon lifetime. @@ -110,6 +110,14 @@ Tool-use strategy (use these aggressively; each call is cheap): default. When a tool_call is REQUIRED (do not skip): +- User asks to modify, edit, update, rewrite, replace, append to, remove + from, or otherwise change a specific file path -> read_file(path) FIRST. + If the path uses ~ or depends on an env var, resolve it with env_get/cwd + as needed, then read the real file before generating any mutating + command or script. Do not emit pseudocode, shell snippets, or commands + that fail to touch the named file. Ground the edit in the file's actual + current contents. If you cannot read the file, ask a focused question or + refuse; do not guess. - User says "the 2 files", "those files", "the log", or any other vague file reference without naming them -> list_dir(path) the relevant directory FIRST to learn the actual names. NEVER emit placeholder