From b955181bb95f3febc50a30e57b6ed39ba532deae Mon Sep 17 00:00:00 2001 From: Michael SCHAL Date: Fri, 2 Oct 2026 23:40:38 +0200 Subject: [PATCH] =?UTF-8?q?Mode=20Code=20:=20une=20=C3=A9criture=20interdi?= =?UTF-8?q?te=20est=20refus=C3=A9e=20d=C3=A8s=20son=20chemin,=20pas=20apr?= =?UTF-8?q?=C3=A8s=20le=20fichier?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Repris du pré-vol d'OpenFox (tool-preflight, 2.0.154). Un write sur un fichier existant jamais lu était refusé par le tracker… une fois tout le fichier généré. builder.md demande des fichiers écrits en entier : un refus pouvait coûter des minutes de décodage pour rien. - Les schémas write, edit, mem_add et mem_edit annoncent le chemin AVANT le contenu. Une map Go sort ses clés par ordre alphabétique : le modèle, qui suit l'ordre du schéma, déroulait tout le contenu avant le chemin. L'interface nomme aussi le fichier dès le début de la frappe. - Dès que le chemin d'un write/edit est complet dans le flux, les gardes du mode Code (fichier lu, chemin permis) sont vérifiées. Refus : la génération est coupée, l'appel réduit à son chemin entre dans l'historique avec le refus pour résultat, et le modèle repart (lire le fichier d'abord). Rien n'est écrit. Co-Authored-By: Claude Opus 5.5 --- internal/loki/code_preflight_test.go | 86 +++++++++++++++++++++++++++ internal/loki/llm_client.go | 89 ++++++++++++++++++++++------ internal/loki/tool_schema_order.go | 50 ++++++++++++++++ 3 files changed, 206 insertions(+), 19 deletions(-) create mode 100644 internal/loki/code_preflight_test.go create mode 100644 internal/loki/tool_schema_order.go diff --git a/internal/loki/code_preflight_test.go b/internal/loki/code_preflight_test.go new file mode 100644 index 0000000..d6c27f0 --- /dev/null +++ b/internal/loki/code_preflight_test.go @@ -0,0 +1,86 @@ +package loki + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "net/url" + "os" + "path/filepath" + "strings" + "sync/atomic" + "testing" + "time" +) + +// Le schéma de write annonce le chemin AVANT le contenu. +func TestSchemaWriteCheminDAbord(t *testing.T) { + b, _ := json.Marshal(writeTool().Function.Parameters) + s := string(b) + if i, j := strings.Index(s, `"file"`), strings.Index(s, `"content"`); i < 0 || j < 0 || i > j { + t.Fatalf("ordre des propriétés : %s", s) + } +} + +// write sur un fichier existant jamais lu : refusé dès que le chemin est +// complet, sans attendre le contenu ; le fichier reste intact. +func TestPreVolCoupeLEcritureRefusee(t *testing.T) { + withWorkspace(t) + cible := filepath.Join(agentCwd(), "main.go") + if err := os.WriteFile(cible, []byte("package main\n"), 0o644); err != nil { + t.Fatal(err) + } + var n int32 + var second []Message + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "text/event-stream") + w.WriteHeader(200) + if atomic.AddInt32(&n, 1) == 1 { + chunk := func(args string) { + esc := strings.ReplaceAll(args, `"`, `\"`) + _, _ = w.Write([]byte(`data: {"choices":[{"delta":{"tool_calls":[{"index":0,"id":"w1","type":"function","function":{"name":"write","arguments":"` + esc + `"}}]}}]}` + "\n\n")) + w.(http.Flusher).Flush() + } + chunk(`{"file":"main.go",`) + chunk(`"content":"package main // début`) + time.Sleep(300 * time.Millisecond) + chunk(` FIN-JAMAIS-LUE"}`) + _, _ = w.Write([]byte(`data: {"choices":[{"delta":{},"finish_reason":"tool_calls"}]}` + "\n\n")) + return + } + var body struct { + Messages []Message `json:"messages"` + } + _ = json.NewDecoder(r.Body).Decode(&body) + second = body.Messages + _, _ = w.Write([]byte(sseChunk("je lis d'abord"))) + _, _ = w.Write([]byte(`data: {"choices":[{"delta":{},"finish_reason":"stop"}]}` + "\n\ndata: [DONE]\n\n")) + })) + t.Cleanup(srv.Close) + u, _ := url.Parse(srv.URL) + if err := SetConfigKey("PORT", u.Port()); err != nil { + t.Fatal(err) + } + var results []string + if _, err := runChat(context.Background(), []Message{{Role: "user", Content: "réécris main.go"}}, 0.7, Caps{Agent: true, Code: true}, func(ev StreamEvent) bool { + if ev.ToolUsed != nil && ev.ToolUsed.Done { + results = append(results, ev.ToolUsed.Result) + } + if ev.ToolUsed != nil && strings.Contains(ev.ToolUsed.Body, "FIN-JAMAIS-LUE") { + t.Error("le contenu a continué d'arriver après le refus") + } + return true + }); err != nil { + t.Fatal(err) + } + if len(results) != 1 || !strings.Contains(results[0], "[refusé]") || !strings.Contains(results[0], "interrompue") { + t.Fatalf("résultats = %q", results) + } + if b, _ := os.ReadFile(cible); string(b) != "package main\n" { + t.Fatalf("fichier modifié : %q", b) + } + if len(second) < 2 || second[len(second)-1].Role != "tool" || !strings.Contains(msgText(second[len(second)-1]), "[refusé]") { + t.Fatalf("le refus n'a pas été rendu au modèle : %+v", second) + } +} diff --git a/internal/loki/llm_client.go b/internal/loki/llm_client.go index 2185abb..44addd9 100644 --- a/internal/loki/llm_client.go +++ b/internal/loki/llm_client.go @@ -99,9 +99,9 @@ func memAddTool() Tool { Description: "Create a memory page. One topic per page, kebab-case name, first line = title (#). Refuses to overwrite an existing page (use mem_edit).", Parameters: map[string]any{ "type": "object", - "properties": map[string]any{ - "file": map[string]any{"type": "string", "description": "Page name"}, - "content": map[string]any{"type": "string", "description": "Markdown, first line = title #"}, + "properties": orderedProps{ + {"file", map[string]any{"type": "string", "description": "Page name"}}, + {"content", map[string]any{"type": "string", "description": "Markdown, first line = title #"}}, }, "required": []string{"file", "content"}, }, @@ -117,10 +117,10 @@ func memEditTool() Tool { Description: "Patch a memory page: old → new, old unique in the page. To append, put the current end of the page in old and the extended version in new.", Parameters: map[string]any{ "type": "object", - "properties": map[string]any{ - "file": map[string]any{"type": "string", "description": "Page name"}, - "old": map[string]any{"type": "string", "description": "Exact text to replace (unique)"}, - "new": map[string]any{"type": "string", "description": "Replacement"}, + "properties": orderedProps{ + {"file", map[string]any{"type": "string", "description": "Page name"}}, + {"old", map[string]any{"type": "string", "description": "Exact text to replace (unique)"}}, + {"new", map[string]any{"type": "string", "description": "Replacement"}}, }, "required": []string{"file", "old", "new"}, }, @@ -136,10 +136,11 @@ func editTool() Tool { Description: "Patch a file by exact replacement: old → new. old must appear EXACTLY once (add context to make it unique). Prefer this over rewriting a whole file.", Parameters: map[string]any{ "type": "object", - "properties": map[string]any{ - "file": map[string]any{"type": "string", "description": "Path"}, - "old": map[string]any{"type": "string", "description": "Exact text to replace (unique)"}, - "new": map[string]any{"type": "string", "description": "Replacement"}, + // Ordre voulu : le chemin d'abord (voir orderedProps). + "properties": orderedProps{ + {"file", map[string]any{"type": "string", "description": "Path"}}, + {"old", map[string]any{"type": "string", "description": "Exact text to replace (unique)"}}, + {"new", map[string]any{"type": "string", "description": "Replacement"}}, }, "required": []string{"file", "old", "new"}, }, @@ -155,9 +156,10 @@ func writeTool() Tool { Description: "Create or replace a file with the exact content given (parent dirs created, content verbatim, no escaping). ALWAYS use this for a script or any text file — NEVER build one through the shell with echo, cat, python -c or Set-Content: quoting breaks.", Parameters: map[string]any{ "type": "object", - "properties": map[string]any{ - "file": map[string]any{"type": "string", "description": "Path"}, - "content": map[string]any{"type": "string", "description": "Full content"}, + // Ordre voulu : le chemin d'abord (voir orderedProps). + "properties": orderedProps{ + {"file", map[string]any{"type": "string", "description": "Path"}}, + {"content", map[string]any{"type": "string", "description": "Full content"}}, }, "required": []string{"file", "content"}, }, @@ -663,22 +665,30 @@ func writeBodyKey(tool string) string { // streaming tool-call arguments JSON, so the UI can show the command being // typed live. Best-effort: it tolerates a truncated tail and basic escapes. func previewArg(args, key string) string { + v, _ := previewArgDone(args, key) + return v +} + +// previewArgDone : comme previewArg, et dit si la valeur est COMPLÈTE (guillemet +// fermant reçu) — de quoi agir sur un argument avant la fin du flux. +func previewArgDone(args, key string) (string, bool) { i := strings.Index(args, "\""+key+"\"") if i < 0 { - return "" + return "", false } rest := args[i+len(key)+2:] if j := strings.Index(rest, ":"); j >= 0 { rest = rest[j+1:] } else { - return "" + return "", false } q := strings.Index(rest, "\"") if q < 0 { - return "" + return "", false } rest = rest[q+1:] var b strings.Builder + closed := false for x := 0; x < len(rest); x++ { c := rest[x] if c == '\\' && x+1 < len(rest) { @@ -699,11 +709,12 @@ func previewArg(args, key string) string { continue } if c == '"' { + closed = true break } b.WriteByte(c) } - return b.String() + return b.String(), closed } // StatsEvent carries llama.cpp's per-completion timing (final chunk). @@ -1201,6 +1212,9 @@ func runChat(ctx context.Context, messages []Message, temperature float64, caps aborted := false // patternHit : flux coupé sur un appel d'outil écrit en texte (voir plus bas). patternHit := false + // preflight : write/edit refusé dès son chemin (voir plus bas) ; non-nil + // mais vide = chemin déjà vérifié, rien à signaler. + var preflight *preflightRefusal for sc.Scan() { line := strings.TrimSpace(sc.Text()) if !strings.HasPrefix(line, "data:") { @@ -1342,6 +1356,20 @@ func runChat(ctx context.Context, messages []Message, temperature float64, caps break } } + // Pré-vol (OpenFox 2.0.154) : dès que le chemin d'un write/edit est + // complet, on vérifie les gardes du mode Code (fichier lu avant + // d'être modifié, chemin permis). Refusé : on coupe la génération + // MAINTENANT au lieu de laisser le modèle écrire tout un fichier + // qui serait de toute façon rejeté. + if caps.Code && preflight == nil && (cur.Function.Name == "write" || cur.Function.Name == "edit") { + if file, done := previewArgDone(cur.Function.Arguments, "file"); done && strings.TrimSpace(file) != "" { + if msg := codeWriteGuard(caps, file, cur.Function.Name == "write"); msg != "" { + preflight = &preflightRefusal{name: cur.Function.Name, id: cur.ID, file: file, msg: msg} + break + } + preflight = &preflightRefusal{} // vérifié : plus besoin de regarder + } + } } continue } @@ -1459,7 +1487,7 @@ func runChat(ctx context.Context, messages []Message, temperature float64, caps // Coupure APRÈS le dernier chunk (finish_reason reçu) : la réponse est // complète, seule la fermeture a raté. On la garde telle quelle. Flux // coupé par NOUS sur un appel écrit en texte : pas une panne non plus. - if scanErr != nil && (finishReason != "" || patternHit) { + if scanErr != nil && (finishReason != "" || patternHit || (preflight != nil && preflight.msg != "")) { scanErr = nil } // Flux coupé vers une API DISTANTE (Wi-Fi, VPN, proxy qui décroche) : on @@ -1504,6 +1532,29 @@ func runChat(ctx context.Context, messages []Message, temperature float64, caps // rapprochée, pas pour tout un long tour d'agent. streamRetries = 0 + // Écriture refusée en plein flux (pré-vol) : l'appel, réduit à son chemin, + // entre dans l'historique avec le refus pour résultat — le modèle voit + // pourquoi, et la boucle repart pour qu'il lise le fichier d'abord. + if preflight != nil && preflight.msg != "" { + id := preflight.id + if id == "" { + id = fmt.Sprintf("call_%d_0", iter) + } + fileArg, _ := json.Marshal(map[string]string{"file": preflight.file}) + tc := ToolCall{ID: id, Type: "function", Function: ToolCallFunc{Name: preflight.name, Arguments: string(fileArg)}} + assistant := Message{Role: "assistant", ToolCalls: []ToolCall{tc}} + if s := assistantContent.String(); s != "" { + assistant.Content = s + } + result := preflight.msg + " (écriture interrompue avant la fin : rien n'a été modifié)" + cb(StreamEvent{ToolUsed: &ToolUsedEvent{Name: preflight.name, Label: preflight.file, Done: true, Result: result}}) + toolMsg := Message{Role: "tool", ToolCallID: id, Content: result} + messages = append(messages, assistant, toolMsg) + extra = append(extra, assistant, toolMsg) + toolRuns++ + continue + } + // Treat any accumulated tool calls as a tool turn even if the backend set // finish_reason to "stop" instead of "tool_calls" (some llama.cpp builds // do this) — otherwise we'd skip execution AND skip answering. diff --git a/internal/loki/tool_schema_order.go b/internal/loki/tool_schema_order.go new file mode 100644 index 0000000..2d73cb8 --- /dev/null +++ b/internal/loki/tool_schema_order.go @@ -0,0 +1,50 @@ +package loki + +import ( + "bytes" + "encoding/json" +) + +// orderedProps : propriétés d'un schéma d'outil sérialisées DANS L'ORDRE donné. +// +// Une map Go sort ses clés par ordre alphabétique : write annonçait content +// AVANT file, edit new avant old. Le modèle écrit ses arguments dans l'ordre +// du schéma — il déroulait donc tout le contenu d'un fichier avant d'en donner +// le chemin. Le chemin d'abord permet de refuser en plein flux une écriture +// interdite (fichier non lu) au lieu d'attendre la fin, et à l'interface de +// nommer le fichier dès le début de la frappe (repris d'OpenFox, 2.0.154). +type orderedProps []propEntry + +type propEntry struct { + Key string + Schema map[string]any +} + +func (o orderedProps) MarshalJSON() ([]byte, error) { + var b bytes.Buffer + b.WriteByte('{') + for i, p := range o { + if i > 0 { + b.WriteByte(',') + } + k, err := json.Marshal(p.Key) + if err != nil { + return nil, err + } + v, err := json.Marshal(p.Schema) + if err != nil { + return nil, err + } + b.Write(k) + b.WriteByte(':') + b.Write(v) + } + b.WriteByte('}') + return b.Bytes(), nil +} + +// preflightRefusal : write/edit refusé pendant le flux, dès que son chemin est +// connu (voir runChat). msg vide = chemin vérifié et accepté. +type preflightRefusal struct { + name, id, file, msg string +}