From 4a97ff5dd165cbacf7ddf5ff86be7ba856937b07 Mon Sep 17 00:00:00 2001 From: weichen Date: Sun, 13 Sep 2026 19:09:48 +0000 Subject: [PATCH] fix(agent): map runner patch paths and require real targets Normalize absolute, ./, and duplicated step prefixes onto output.md, and fail closed when the runner reports ok without a non-empty deliverable. Co-authored-by: Cursor --- internal/agent/backend_test.go | 85 +++++++++++++++++++++++ internal/agent/deliverable.go | 104 +++++++++++++++++++++++++++++ internal/agent/deliverable_test.go | 83 +++++++++++++++++++++++ internal/agent/piagent.go | 36 +++++----- internal/squad/workflow.go | 17 +++-- 5 files changed, 297 insertions(+), 28 deletions(-) create mode 100644 internal/agent/deliverable.go create mode 100644 internal/agent/deliverable_test.go diff --git a/internal/agent/backend_test.go b/internal/agent/backend_test.go index ffaced36..cb082435 100644 --- a/internal/agent/backend_test.go +++ b/internal/agent/backend_test.go @@ -7,6 +7,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "time" @@ -101,3 +102,87 @@ func TestPiAgentRemoteRunner(t *testing.T) { t.Errorf("unexpected local file content: %s", string(localContent)) } } + +func TestPiAgentRemoteRunner_PathAliasesAndMissing(t *testing.T) { + run := func(mode, want string, expectOK bool) { + t.Helper() + dir := filepath.Join(t.TempDir(), "tasks", "01_draft") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/run" { + http.NotFound(w, r) + return + } + var patches []map[string]string + switch mode { + case "abs": + patches = []map[string]string{{ + "rel_path": filepath.Join(dir, "output.md"), + "content": "# from absolute rel_path\n", + }} + case "nested": + patches = []map[string]string{{ + "rel_path": "tasks/01_draft/output.md", + "content": "# from nested rel_path\n", + }} + case "dot": + patches = []map[string]string{{ + "rel_path": "./output.md", + "content": "# from dot rel_path\n", + }} + default: + patches = []map[string]string{{ + "rel_path": "execution.log", + "content": "Outcome: SUCCESS\n", + }} + } + _ = json.NewEncoder(w).Encode(map[string]any{"ok": true, "patches": patches}) + })) + defer ts.Close() + + cfg := &config.Config{ + AgentBackend: "pi-agent", + PiAgentRunnerURL: ts.URL, + PiAgentModel: "MiniMax-M3", + PiAgentProvider: "minimax", + AgentTimeout: 5 * time.Second, + } + res, err := agent.NewBackend("pi-agent", cfg).Execute(context.Background(), agent.Request{ + WorkDir: dir, + Prompt: "draft", + Targets: []string{"output.md"}, + }) + if err != nil { + t.Fatalf("mode %s unexpected err: %v", mode, err) + } + if expectOK { + if !res.Success { + t.Fatalf("mode %s want success, got %s", mode, res.ErrorReason) + } + got, rerr := os.ReadFile(filepath.Join(dir, "output.md")) + if rerr != nil { + t.Fatalf("mode %s missing file: %v", mode, rerr) + } + if string(got) != want { + t.Fatalf("mode %s content %q", mode, got) + } + return + } + if res.Success { + t.Fatalf("mode %s must fail closed", mode) + } + if !strings.Contains(res.ErrorReason, "missing target deliverable output.md") { + t.Fatalf("mode %s reason %q", mode, res.ErrorReason) + } + if _, serr := os.Stat(filepath.Join(dir, "output.md")); serr == nil { + t.Fatalf("mode %s must not invent output.md", mode) + } + } + + run("abs", "# from absolute rel_path\n", true) + run("nested", "# from nested rel_path\n", true) + run("dot", "# from dot rel_path\n", true) + run("missing", "", false) +} diff --git a/internal/agent/deliverable.go b/internal/agent/deliverable.go new file mode 100644 index 00000000..13fe7748 --- /dev/null +++ b/internal/agent/deliverable.go @@ -0,0 +1,104 @@ +package agent + +import ( + "bytes" + "fmt" + "os" + "path/filepath" + "strings" +) + +func slashClean(p string) string { + return filepath.ToSlash(filepath.Clean(p)) +} + +// MatchDeliverableRel reports whether a runner rel_path refers to target +// inside workDir. Accepts output.md, ./output.md, absolute paths under workDir, +// and a duplicated step prefix (tasks/01_draft/output.md when workDir already +// ends with tasks/01_draft). Paths that escape workDir never match. +func MatchDeliverableRel(workDir, relPath, target string) bool { + target = slashClean(strings.TrimSpace(target)) + if target == "" || target == "." || strings.HasPrefix(target, "../") || strings.Contains(target, "..") { + return false + } + raw := strings.TrimSpace(relPath) + if raw == "" { + return false + } + abs := raw + if !filepath.IsAbs(raw) { + abs = filepath.Join(workDir, raw) + } + abs = filepath.Clean(abs) + rel, err := filepath.Rel(filepath.Clean(workDir), abs) + if err != nil { + return false + } + rel = slashClean(rel) + if rel == ".." || strings.HasPrefix(rel, "../") { + return false + } + if rel == target { + return true + } + if filepath.Base(rel) != filepath.Base(target) { + return false + } + prefix := slashClean(filepath.Dir(rel)) + wd := slashClean(workDir) + return prefix != "." && (strings.HasSuffix(wd, "/"+prefix) || strings.HasSuffix(wd, prefix)) +} + +// CollectTargetPatches maps incoming runner keys onto canonical target names +// and also accepts a non-empty file already at workDir/target. Empty bodies +// do not count — callers must not invent a deliverable. +func CollectTargetPatches(workDir string, targets []string, incoming map[string][]byte) map[string][]byte { + out := make(map[string][]byte, len(targets)) + for _, t := range targets { + t = strings.TrimSpace(t) + if t == "" { + continue + } + for k, v := range incoming { + if len(bytes.TrimSpace(v)) == 0 { + continue + } + if MatchDeliverableRel(workDir, k, t) { + out[t] = v + break + } + } + if _, ok := out[t]; ok { + continue + } + p := filepath.Join(workDir, t) + if b, err := os.ReadFile(p); err == nil && len(bytes.TrimSpace(b)) > 0 { + out[t] = b + } + } + return out +} + +// MaterializeTargets writes collected non-empty target bodies to workDir/. +// Returns a missing-target error without writing a stub if any required file is absent. +func MaterializeTargets(workDir string, targets []string, incoming map[string][]byte) (map[string][]byte, error) { + collected := CollectTargetPatches(workDir, targets, incoming) + for _, t := range targets { + t = strings.TrimSpace(t) + if t == "" { + continue + } + body, ok := collected[t] + if !ok || len(bytes.TrimSpace(body)) == 0 { + return collected, fmt.Errorf("missing target deliverable %s", t) + } + dest := filepath.Join(workDir, filepath.FromSlash(t)) + if err := os.MkdirAll(filepath.Dir(dest), 0o755); err != nil { + return collected, err + } + if err := os.WriteFile(dest, body, 0o644); err != nil { + return collected, err + } + } + return collected, nil +} diff --git a/internal/agent/deliverable_test.go b/internal/agent/deliverable_test.go new file mode 100644 index 00000000..2b4ea7dc --- /dev/null +++ b/internal/agent/deliverable_test.go @@ -0,0 +1,83 @@ +package agent + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestMatchDeliverableRel_Aliases(t *testing.T) { + workDir := filepath.Join(string(filepath.Separator), "app", "data", "workspaces", "bizdev", "prelim_x", "tasks", "01_draft") + abs := filepath.Join(workDir, "output.md") + cases := []struct { + rel string + want bool + }{ + {"output.md", true}, + {"./output.md", true}, + {abs, true}, + {"tasks/01_draft/output.md", true}, + {"execution.log", false}, + {"../output.md", false}, + {"/etc/passwd", false}, + {"tasks/02_review/output.md", false}, + } + for _, tc := range cases { + if got := MatchDeliverableRel(workDir, tc.rel, "output.md"); got != tc.want { + t.Errorf("MatchDeliverableRel(%q) = %v, want %v", tc.rel, got, tc.want) + } + } +} + +func TestMaterializeTargets_MapsAbsoluteAndNested(t *testing.T) { + workDir := filepath.Join(t.TempDir(), "tasks", "01_draft") + if err := os.MkdirAll(workDir, 0o755); err != nil { + t.Fatal(err) + } + body := []byte("# 系统初步设计方案\n\n真实 runner 产物\n") + incoming := map[string][]byte{ + filepath.Join(workDir, "output.md"): body, + } + got, err := MaterializeTargets(workDir, []string{"output.md"}, incoming) + if err != nil { + t.Fatalf("absolute rel_path must materialize: %v", err) + } + if string(got["output.md"]) != string(body) { + t.Fatalf("canonical patch missing") + } + onDisk, err := os.ReadFile(filepath.Join(workDir, "output.md")) + if err != nil { + t.Fatal(err) + } + if string(onDisk) != string(body) { + t.Fatalf("disk output.md mismatch") + } + + nestedDir := filepath.Join(t.TempDir(), "ws", "tasks", "01_draft") + if err := os.MkdirAll(nestedDir, 0o755); err != nil { + t.Fatal(err) + } + if _, err := MaterializeTargets(nestedDir, []string{"output.md"}, map[string][]byte{ + "tasks/01_draft/output.md": []byte("# nested alias\n"), + }); err != nil { + t.Fatalf("duplicated step prefix must map: %v", err) + } +} + +func TestMaterializeTargets_MissingAndEmptyFail(t *testing.T) { + workDir := t.TempDir() + if _, err := MaterializeTargets(workDir, []string{"output.md"}, map[string][]byte{ + "execution.log": []byte("Outcome: SUCCESS\n"), + }); err == nil || !strings.Contains(err.Error(), "missing target deliverable output.md") { + t.Fatalf("must fail closed without inventing output.md, got %v", err) + } + if _, err := os.Stat(filepath.Join(workDir, "output.md")); err == nil { + t.Fatal("must not write a stub output.md") + } + if _, err := MaterializeTargets(workDir, []string{"output.md"}, map[string][]byte{ + "output.md": []byte(" \n"), + }); err == nil { + t.Fatal("whitespace-only body must not count as a deliverable") + } +} diff --git a/internal/agent/piagent.go b/internal/agent/piagent.go index 8d91d50e..389a4856 100644 --- a/internal/agent/piagent.go +++ b/internal/agent/piagent.go @@ -106,12 +106,14 @@ func (p *PiAgentBackend) Execute(ctx context.Context, req Request) (*Result, err }, nil } - patches := make(map[string][]byte) - for _, t := range req.Targets { - p := filepath.Join(workDir, t) - if b, err := os.ReadFile(p); err == nil { - patches[t] = b - } + patches, err := MaterializeTargets(workDir, req.Targets, nil) + if err != nil { + return &Result{ + Success: false, + ErrorReason: err.Error(), + Patches: patches, + Logs: stdout.String(), + }, nil } return &Result{ @@ -182,21 +184,17 @@ func (p *PiAgentBackend) executeRemote(ctx context.Context, runnerURL string, re }, nil } - patches := make(map[string][]byte) + incoming := make(map[string][]byte, len(remoteResp.Patches)) for _, pt := range remoteResp.Patches { - patches[pt.RelPath] = []byte(pt.Content) - localPath := filepath.Join(workDir, pt.RelPath) - _ = os.MkdirAll(filepath.Dir(localPath), 0755) - _ = os.WriteFile(localPath, []byte(pt.Content), 0644) + incoming[pt.RelPath] = []byte(pt.Content) } - - for _, t := range req.Targets { - if _, ok := patches[t]; !ok { - targetPath := filepath.Join(workDir, t) - if b, err := os.ReadFile(targetPath); err == nil { - patches[t] = b - } - } + patches, matErr := MaterializeTargets(workDir, req.Targets, incoming) + if matErr != nil { + return &Result{ + Success: false, + ErrorReason: matErr.Error(), + Patches: patches, + }, nil } return &Result{ diff --git a/internal/squad/workflow.go b/internal/squad/workflow.go index a38a379c..91f6170a 100644 --- a/internal/squad/workflow.go +++ b/internal/squad/workflow.go @@ -583,16 +583,15 @@ func (d *WorkflowDriver) runOneStep( return fmt.Errorf("step %s failed: %s", step.ID, errMsg) } - // 2. 确认 deliverable 文件落盘 - targetPath := filepath.Join(subDir, deliverableName) - if _, err := os.Stat(targetPath); err != nil { - // 若后端补丁在内存中返回，写入磁盘 - if res != nil && len(res.Patches[deliverableName]) > 0 { - _ = os.WriteFile(targetPath, res.Patches[deliverableName], 0o644) - } else { - return fmt.Errorf("step %s missing target deliverable %s", step.ID, deliverableName) - } + // 2. 确认 deliverable 文件落盘（路径别名归一化；缺产物不得编造） + incoming := map[string][]byte{} + if res != nil { + incoming = res.Patches } + if _, matErr := agent.MaterializeTargets(subDir, []string{deliverableName}, incoming); matErr != nil { + return fmt.Errorf("step %s %w", step.ID, matErr) + } + targetPath := filepath.Join(subDir, deliverableName) // 3. 检查是否有 fields.json 并合并入 blackboard.json fieldsPath := filepath.Join(subDir, "fields.json")