fix(service): keep explicit backend failure fail-closed
A this-invocation output.md is preserved and never retried over, but Success=false stays a step failure so truncated writes cannot pass. Prior-round residue without a trusted patch is also not success. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -40,7 +40,7 @@ mutex: document-revision-prelim-output
|
||||
4. [x] 纳入现场时序:write 已流出但 `terminated`;SDK retry 1–3 Connection error;`auto_retry_end success=false`;仍 `agent_settled`;trace `success=true`
|
||||
5. [x] settle-policy + Go 分类 + `runOneStep` 有界全新重试 + 单测
|
||||
6. [x] `.dockerignore` 放行 `templates/**/*.md`;Docker context 实测 `/app/templates/docs/prelim_design.md`
|
||||
7. [x] 已有目标不覆盖;仅 provider+缺文件或 runner 明确失败才重试
|
||||
7. [x] 本次写入的目标不覆盖、不再调用;backend 明确失败仍 FAIL(不把文件存在当成功);上一轮残留不当成功
|
||||
8. [ ] 占槽后真实 UI 重跑(本轮不做)
|
||||
|
||||
## 验收标准
|
||||
|
||||
@@ -27,7 +27,7 @@ updated: 2026-09-13
|
||||
| Wave 0 | Runner `rel_path` 归一化;缺产物则 `Success=false` | 绝对 / `./` / 重复子目录能落盘;空 patches 失败 |
|
||||
| Wave 0 | `runner.mjs`:`auto_retry_end` 失败或目标不存在不得 `ok:true` | 与现场 `agent_settled` 假成功对齐 |
|
||||
| Wave 1 | `settle-policy`:`terminated` / `Connection error` / `auto_retry_end` 后 settle 失败,返回 `retryable` | 干净 settle + 缺文件 → `retryable=false` |
|
||||
| Wave 1 | Go `AssessStepFailure` + `runOneStep` 有界全新 invocation(≤2) | 仅 `(provider terminated/connection + 缺目标)` 或 runner 明确失败;已有目标不覆盖;永久缺失 1 次;耗尽 fail-closed |
|
||||
| Wave 1 | Go `AssessStepFailure` + `runOneStep` 有界全新 invocation(≤2) | 仅 `(provider terminated/connection + 缺目标)` 或 runner 明确失败;本次写入不覆盖且失败不翻成功;上一轮残留不当成功;耗尽 fail-closed |
|
||||
| Wave 1 | 旧 runner 假成功:扫描 `execution.log` 识别 settled failure | 不依赖本轮 env-service 重建 |
|
||||
|
||||
## 取舍
|
||||
|
||||
@@ -44,8 +44,9 @@ type StepFailureSignals struct {
|
||||
WorkDir string
|
||||
ExtraText string
|
||||
TargetMissing bool
|
||||
// TargetExists is a non-empty deliverable already on disk. Retry must
|
||||
// not overwrite it.
|
||||
// TargetExists means this invocation wrote a non-empty file that differs
|
||||
// from the pre-invoke baseline. Do not retry or overwrite it. The step
|
||||
// still fails unless the backend already reported a trusted success.
|
||||
TargetExists bool
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package squad
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"os"
|
||||
"path/filepath"
|
||||
@@ -235,5 +236,9 @@ func (b *testEchoBackend) Execute(ctx context.Context, req agent.Request) (*agen
|
||||
if _, err := os.Stat(p); os.IsNotExist(err) {
|
||||
_ = os.WriteFile(p, []byte("## 步骤执行交付产物\n\n已完成任务要求论证与数据沉淀。\n"), 0o644)
|
||||
}
|
||||
return &agent.Result{Success: true}, nil
|
||||
content, err := os.ReadFile(p)
|
||||
if err != nil || len(bytes.TrimSpace(content)) == 0 {
|
||||
return &agent.Result{Success: false, ErrorReason: "missing target deliverable " + target}, nil
|
||||
}
|
||||
return &agent.Result{Success: true, Patches: map[string][]byte{target: content}}, nil
|
||||
}
|
||||
|
||||
@@ -71,6 +71,11 @@ func (b *countingBackend) Execute(ctx context.Context, req agent.Request) (*agen
|
||||
_ = os.WriteFile(filepath.Join(req.WorkDir, "output.md"), body, 0o644)
|
||||
return &agent.Result{Success: false, ErrorReason: "Connection error.", Retryable: true}, nil
|
||||
|
||||
case "success-without-patch":
|
||||
return &agent.Result{Success: true, Patches: map[string][]byte{
|
||||
"execution.log": []byte("Outcome: SUCCESS\n"),
|
||||
}}, nil
|
||||
|
||||
default:
|
||||
return &agent.Result{Success: false, ErrorReason: "unknown mode"}, nil
|
||||
}
|
||||
@@ -174,14 +179,14 @@ func TestWorkflowDriver_DoesNotOverwriteExistingTarget(t *testing.T) {
|
||||
wsDir, driver := prepareSingleWriterWS(t)
|
||||
backend := &countingBackend{mode: "fail-after-writing-target"}
|
||||
res, err := driver.Execute(context.Background(), wsDir, backend, squad.ExecuteOptions{Timeout: 5 * time.Second})
|
||||
if err != nil {
|
||||
t.Fatalf("existing target must be accepted, not retried: %v", err)
|
||||
}
|
||||
if !res.Success {
|
||||
t.Fatalf("want success from existing file, got %s", res.ErrorReason)
|
||||
if err == nil || res.Success {
|
||||
t.Fatal("backend explicit failure must fail-closed; a possibly truncated file is not success")
|
||||
}
|
||||
if backend.calls != 1 {
|
||||
t.Fatalf("must not re-invoke over an existing target, calls=%d", backend.calls)
|
||||
t.Fatalf("must not re-invoke over this-invocation target, calls=%d", backend.calls)
|
||||
}
|
||||
if !strings.Contains(err.Error(), "Connection error.") {
|
||||
t.Fatalf("want backend failure preserved, got %v", err)
|
||||
}
|
||||
body, err := os.ReadFile(filepath.Join(wsDir, "tasks", "01_draft", "output.md"))
|
||||
if err != nil {
|
||||
@@ -192,6 +197,29 @@ func TestWorkflowDriver_DoesNotOverwriteExistingTarget(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkflowDriver_PriorResidueIsNotSuccess(t *testing.T) {
|
||||
wsDir, driver := prepareSingleWriterWS(t)
|
||||
leftover := []byte("# leftover from previous round\n")
|
||||
if err := os.WriteFile(filepath.Join(wsDir, "tasks", "01_draft", "output.md"), leftover, 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
backend := &countingBackend{mode: "success-without-patch"}
|
||||
res, err := driver.Execute(context.Background(), wsDir, backend, squad.ExecuteOptions{Timeout: 5 * time.Second})
|
||||
if err == nil || res.Success {
|
||||
t.Fatal("prior-round residue must not count as this-invocation success")
|
||||
}
|
||||
if backend.calls != 1 {
|
||||
t.Fatalf("residue without trusted patch is a permanent contract miss, calls=%d", backend.calls)
|
||||
}
|
||||
got, err := os.ReadFile(filepath.Join(wsDir, "tasks", "01_draft", "output.md"))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if string(got) != string(leftover) {
|
||||
t.Fatalf("residue must stay on disk unchanged: %s", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestWorkflowDriver_BoundedTransientRetryThenFailClosed(t *testing.T) {
|
||||
wsDir, driver := prepareSingleWriterWS(t)
|
||||
backend := &countingBackend{mode: "always-transient", failText: "Connection error."}
|
||||
|
||||
+37
-19
@@ -1,6 +1,7 @@
|
||||
package squad
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
@@ -512,13 +513,9 @@ func (d *WorkflowDriver) runOneStep(
|
||||
)
|
||||
for attempt := 1; attempt <= agent.MaxTransientStepInvocations; attempt++ {
|
||||
onDisk, _ := os.ReadFile(prevOutPath)
|
||||
produced := len(strings.TrimSpace(string(onDisk))) > 0 && string(onDisk) != prevContent
|
||||
if attempt > 1 && produced {
|
||||
ok = true
|
||||
matErr = nil
|
||||
if res != nil {
|
||||
res.Success = true
|
||||
}
|
||||
wroteThisInvocation := deliverableWroteThisInvocation(prevContent, onDisk)
|
||||
if attempt > 1 && wroteThisInvocation {
|
||||
// Preserve a this-invocation file (may be truncated). Do not call again.
|
||||
break
|
||||
}
|
||||
if attempt > 1 && streamer != nil {
|
||||
@@ -536,32 +533,34 @@ func (d *WorkflowDriver) runOneStep(
|
||||
if res != nil {
|
||||
incoming = res.Patches
|
||||
}
|
||||
hasTrustedPatch := incomingHasDeliverable(subDir, deliverableName, incoming)
|
||||
|
||||
if execErr == nil && res != nil && res.Success {
|
||||
if _, matErr = agent.MaterializeTargets(subDir, []string{deliverableName}, incoming); matErr != nil {
|
||||
res.Success = false
|
||||
res.ErrorReason = matErr.Error()
|
||||
} else if !hasTrustedPatch {
|
||||
after, _ := os.ReadFile(prevOutPath)
|
||||
if !deliverableWroteThisInvocation(prevContent, after) {
|
||||
matErr = fmt.Errorf("missing target deliverable %s", deliverableName)
|
||||
res.Success = false
|
||||
res.ErrorReason = matErr.Error()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
onDisk, _ = os.ReadFile(prevOutPath)
|
||||
produced = len(strings.TrimSpace(string(onDisk))) > 0 && string(onDisk) != prevContent
|
||||
wroteThisInvocation = deliverableWroteThisInvocation(prevContent, onDisk)
|
||||
newContent := ""
|
||||
if produced {
|
||||
newContent = string(onDisk)
|
||||
} else if res != nil {
|
||||
if hasTrustedPatch && res != nil {
|
||||
if b, has := res.Patches[deliverableName]; has {
|
||||
newContent = string(b)
|
||||
}
|
||||
}
|
||||
if newContent == "" {
|
||||
if newContent == "" && wroteThisInvocation {
|
||||
newContent = string(onDisk)
|
||||
}
|
||||
ok = (execErr == nil && res != nil && res.Success && matErr == nil) || produced
|
||||
if produced && res != nil {
|
||||
res.Success = true
|
||||
matErr = nil
|
||||
}
|
||||
ok = execErr == nil && res != nil && res.Success && matErr == nil
|
||||
sim := OutputSimilarity(prevContent, newContent)
|
||||
if mu != nil {
|
||||
mu.Lock()
|
||||
@@ -604,8 +603,8 @@ func (d *WorkflowDriver) runOneStep(
|
||||
Err: execErr,
|
||||
Result: res,
|
||||
WorkDir: subDir,
|
||||
TargetMissing: !produced && (matErr != nil || (res != nil && strings.Contains(res.ErrorReason, "missing target deliverable"))),
|
||||
TargetExists: produced,
|
||||
TargetMissing: !wroteThisInvocation && (matErr != nil || (res != nil && strings.Contains(res.ErrorReason, "missing target deliverable"))),
|
||||
TargetExists: wroteThisInvocation,
|
||||
})
|
||||
if assess.Class != agent.FailureTransient || attempt >= agent.MaxTransientStepInvocations {
|
||||
break
|
||||
@@ -711,6 +710,25 @@ func (d *WorkflowDriver) runOneStep(
|
||||
return nil
|
||||
}
|
||||
|
||||
func deliverableWroteThisInvocation(baseline string, onDisk []byte) bool {
|
||||
if len(bytes.TrimSpace(onDisk)) == 0 {
|
||||
return false
|
||||
}
|
||||
return string(onDisk) != baseline
|
||||
}
|
||||
|
||||
func incomingHasDeliverable(workDir, target string, incoming map[string][]byte) bool {
|
||||
for k, v := range incoming {
|
||||
if len(bytes.TrimSpace(v)) == 0 {
|
||||
continue
|
||||
}
|
||||
if agent.MatchDeliverableRel(workDir, k, target) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func (d *WorkflowDriver) applyDynamicAdjustment(
|
||||
ctx context.Context,
|
||||
wsDir string,
|
||||
|
||||
Reference in New Issue
Block a user