fix: make h2 patch repair failures diagnosable
This commit is contained in:
thanhnv
@@ -59,6 +59,7 @@ export interface GoalJob {
|
||||
context_manifest?: { files: number; characters: number; truncated: boolean; path?: string };
|
||||
approval?: { id: string; status: string; action: string };
|
||||
reviewer_attempts?: GoalReviewerAttempt[];
|
||||
patch_repair_attempts?: GoalPatchRepairAttempt[];
|
||||
patch_artifact?: { path: string; sha256: string; files: string[]; bytes: number; status: string; preview?: string; approval_id?: string; applied_at?: string; applied_by?: string };
|
||||
verification?: Array<{ command: string; exit_code: number; output: string }>;
|
||||
}
|
||||
@@ -75,6 +76,14 @@ export interface GoalReviewerAttempt {
|
||||
latency_ms: number;
|
||||
}
|
||||
|
||||
export interface GoalPatchRepairAttempt {
|
||||
attempt: number;
|
||||
provider: string;
|
||||
model: string;
|
||||
status: 'pass' | 'failed';
|
||||
reason: string;
|
||||
}
|
||||
|
||||
interface ModelConnection {
|
||||
id: string;
|
||||
kind: 'local' | 'cloud' | 'gateway';
|
||||
|
||||
@@ -313,6 +313,7 @@ export interface GoalJob {
|
||||
context_manifest?: { files: number; characters: number; truncated: boolean; path?: string };
|
||||
approval?: { id: string; status: string; action: string };
|
||||
reviewer_attempts?: GoalReviewerAttempt[];
|
||||
patch_repair_attempts?: GoalPatchRepairAttempt[];
|
||||
patch_artifact?: { path: string; sha256: string; files: string[]; bytes: number; status: string; preview?: string; approval_id?: string; applied_at?: string; applied_by?: string };
|
||||
verification?: Array<{ command: string; exit_code: number; output: string }>;
|
||||
}
|
||||
@@ -329,6 +330,14 @@ export interface GoalReviewerAttempt {
|
||||
latency_ms: number;
|
||||
}
|
||||
|
||||
export interface GoalPatchRepairAttempt {
|
||||
attempt: number;
|
||||
provider: string;
|
||||
model: string;
|
||||
status: 'pass' | 'failed';
|
||||
reason: string;
|
||||
}
|
||||
|
||||
export interface GoalProject {
|
||||
project_id: string;
|
||||
domain: string;
|
||||
|
||||
@@ -200,6 +200,7 @@ export function Goals() {
|
||||
</div>
|
||||
)}
|
||||
{selected.error && <div className="mt-4 rounded-xl border border-rose-200 bg-rose-50 p-3 text-sm text-rose-700">{selected.error}</div>}
|
||||
{selected.patch_repair_attempts && selected.patch_repair_attempts.length > 0 && <details open className="mt-4 overflow-hidden rounded-xl border border-amber-200 bg-amber-50/50"><summary className="cursor-pointer px-4 py-3 text-sm font-semibold text-amber-950">H2 patch-repair ledger ({selected.patch_repair_attempts.length})</summary><div className="space-y-2 border-t border-amber-200 p-4">{selected.patch_repair_attempts.map((attempt) => <div key={`${attempt.attempt}-${attempt.model}`} className="rounded-lg border border-amber-100 bg-white px-3 py-2.5 text-xs"><div className="flex flex-wrap items-center justify-between gap-2"><span className="font-semibold text-slate-700">Attempt {attempt.attempt}</span><span className="font-mono text-slate-500">{attempt.provider} · {attempt.model}</span><StatusBadge value={attempt.status} /></div><div className="mt-1.5 break-words font-mono text-[11px] leading-5 text-slate-600">{attempt.reason}</div></div>)}</div></details>}
|
||||
{selected.patch_artifact && <details open={selected.status === 'requires_approval'} className="mt-5 overflow-hidden rounded-xl border border-violet-200 bg-violet-50/40"><summary className="cursor-pointer px-4 py-3 text-sm font-semibold text-violet-950">Reviewed implementation patch · {selected.patch_artifact.files.length} file(s)</summary><div className="border-t border-violet-200 p-4"><div className="mb-3 flex flex-wrap gap-2">{selected.patch_artifact.files.map((file) => <span key={file} className="rounded-full bg-white px-2.5 py-1 font-mono text-[11px] text-violet-800 shadow-sm">{file}</span>)}</div><pre className="max-h-96 overflow-auto rounded-xl bg-slate-950 p-4 text-xs leading-5 text-slate-100">{selected.patch_artifact.preview || 'Patch preview unavailable'}</pre><div className="mt-3 font-mono text-[10px] text-violet-600">SHA-256 {selected.patch_artifact.sha256}</div></div></details>}
|
||||
{selected.approval && selected.status === 'requires_approval' && <div className="mt-4 rounded-xl border border-amber-200 bg-amber-50 p-4 text-sm text-amber-950"><div className="font-semibold">Reviewed patch awaiting approval</div><p className="mt-1 leading-6">Proposal <span className="font-mono">{selected.approval.id}</span> requires an approver different from proposer <span className="font-mono">{selected.actor}</span>.</p><div className="mt-4 grid gap-3 sm:grid-cols-2"><label><span className="text-xs font-semibold">Approver identity</span><input value={approver} onChange={(event) => setApprover(event.target.value)} className="mt-1 w-full rounded-lg border border-amber-300 bg-white px-3 py-2 text-sm outline-none focus:ring-2 focus:ring-amber-500" /></label><label><span className="text-xs font-semibold">Decision reason</span><input value={approvalReason} onChange={(event) => setApprovalReason(event.target.value)} className="mt-1 w-full rounded-lg border border-amber-300 bg-white px-3 py-2 text-sm outline-none focus:ring-2 focus:ring-amber-500" /></label></div><div className="mt-4 flex flex-wrap gap-2"><button type="button" disabled={approveAndApply.isPending || retryApply.isPending || approver.trim().length < 3 || approver.trim() === selected.actor || approvalReason.trim().length < 5} onClick={() => approveAndApply.mutate(selected)} className="rounded-lg bg-amber-700 px-4 py-2 text-xs font-semibold text-white transition hover:bg-amber-800 disabled:cursor-not-allowed disabled:bg-amber-300">{approveAndApply.isPending ? 'Approving, applying and verifying…' : 'Approve & Apply patch'}</button><button type="button" disabled={approveAndApply.isPending || retryApply.isPending || approver.trim().length < 3} onClick={() => retryApply.mutate(selected)} className="rounded-lg border border-amber-300 bg-white px-3 py-2 text-xs font-semibold text-amber-900 hover:bg-amber-100 disabled:text-amber-300">{retryApply.isPending ? 'Applying…' : 'Retry already-approved patch'}</button><Link to="/approvals" className="inline-flex rounded-lg border border-amber-300 bg-white px-3 py-2 text-xs font-semibold text-amber-900 hover:bg-amber-100">Open approval inbox</Link></div>{(approveAndApply.isError || retryApply.isError) && <div role="alert" className="mt-3 rounded-lg border border-rose-200 bg-rose-50 p-3 text-xs font-medium text-rose-700">{errorMessage(approveAndApply.error || retryApply.error)}</div>}</div>}
|
||||
{selected.verification && selected.verification.length > 0 && <div className="mt-4 rounded-xl border border-emerald-200 bg-emerald-50 p-4"><div className="text-sm font-semibold text-emerald-900">Patch applied and verified</div>{selected.verification.map((check) => <div key={check.command} className="mt-2 flex items-center justify-between gap-3 rounded-lg bg-white px-3 py-2 text-xs"><span className="font-mono text-emerald-800">{check.command}</span><StatusBadge value={check.exit_code === 0 ? 'pass' : 'failed'} /></div>)}</div>}
|
||||
|
||||
@@ -299,9 +299,9 @@ def validate_write_output(text: str) -> str:
|
||||
def patch_repair_attempts() -> int:
|
||||
"""Return the bounded number of chances to repair an invalid model diff."""
|
||||
try:
|
||||
configured = int(os.environ.get("CASAN_GOAL_PATCH_REPAIR_ATTEMPTS", "2"))
|
||||
configured = int(os.environ.get("CASAN_GOAL_PATCH_REPAIR_ATTEMPTS", "3"))
|
||||
except ValueError:
|
||||
configured = 2
|
||||
configured = 3
|
||||
return min(max(configured, 0), 3)
|
||||
|
||||
|
||||
@@ -356,20 +356,45 @@ def repair_write_output(model: str, original_prompt: str, invalid_output: str, c
|
||||
)
|
||||
candidates = patch_repair_models(model)[:patch_repair_attempts()]
|
||||
last_metadata, last_reason = {}, "goal_patch_missing"
|
||||
for candidate in candidates:
|
||||
attempts = []
|
||||
for attempt_number, candidate in enumerate(candidates, start=1):
|
||||
ok, output, metadata, reason = call_model(candidate, repair_prompt, candidate.startswith(("openai:", "anthropic:", "openai-compatible:")), max_output_tokens=patch_output_tokens())
|
||||
metadata = dict(metadata)
|
||||
metadata["repair_model"] = candidate
|
||||
metadata["repair_attempt"] = attempt_number
|
||||
last_metadata = metadata
|
||||
if not ok:
|
||||
last_reason = f"goal_patch_repair_failed:{reason}"
|
||||
attempts.append({
|
||||
"attempt": attempt_number,
|
||||
"provider": provider_for_model(candidate),
|
||||
"model": candidate,
|
||||
"status": "failed",
|
||||
"reason": last_reason,
|
||||
})
|
||||
continue
|
||||
try:
|
||||
validate_write_output(output)
|
||||
except ValueError as error:
|
||||
last_reason = str(error)
|
||||
attempts.append({
|
||||
"attempt": attempt_number,
|
||||
"provider": provider_for_model(candidate),
|
||||
"model": candidate,
|
||||
"status": "failed",
|
||||
"reason": last_reason,
|
||||
})
|
||||
continue
|
||||
attempts.append({
|
||||
"attempt": attempt_number,
|
||||
"provider": provider_for_model(candidate),
|
||||
"model": candidate,
|
||||
"status": "pass",
|
||||
"reason": "ok",
|
||||
})
|
||||
metadata["repair_attempts"] = attempts
|
||||
return True, output, metadata, "ok"
|
||||
last_metadata["repair_attempts"] = attempts
|
||||
return False, "", last_metadata, f"goal_patch_repair_invalid:{last_reason}"
|
||||
|
||||
|
||||
@@ -785,8 +810,9 @@ def run(job_path: str) -> int:
|
||||
reason = f"goal_patch_repair_invalid:{repair_error}"
|
||||
if reason:
|
||||
actual_repair_model = str(repaired_meta.get("repair_model") or repair_model)
|
||||
update_job(job_path, local_usage=local_meta, patch_repair_attempts=repaired_meta.get("repair_attempts", []))
|
||||
stage(job_path, "local-worker", "error", reason, provider_for_model(actual_repair_model), actual_repair_model)
|
||||
emit(goal_id, "H2-tool", "error", "Worker violated patch output contract after repair", {"reason": reason, "repair_provider": provider_for_model(actual_repair_model), "repair_model": actual_repair_model})
|
||||
emit(goal_id, "H2-tool", "error", "Worker violated patch output contract after repair", {"reason": reason, "repair_provider": provider_for_model(actual_repair_model), "repair_model": actual_repair_model, "repair_attempts": repaired_meta.get("repair_attempts", [])})
|
||||
raise ValueError(reason)
|
||||
stage(job_path, "local-worker", "pass", "Primary solution prepared", job.get("local_provider", ""), local_model)
|
||||
update_job(job_path, local_draft=safe_local, local_usage=local_meta)
|
||||
|
||||
@@ -67,6 +67,37 @@ def generation_max_tokens(role):
|
||||
return min(max(configured, 64), 8192)
|
||||
|
||||
|
||||
def decode_provider_json(response, provider):
|
||||
"""Decode a provider response without mislabelling an empty 2xx body.
|
||||
|
||||
Gateways occasionally terminate an upstream stream after accepting a
|
||||
request and return an empty (or HTML) 2xx response. Treating that as a
|
||||
generic JSONDecodeError makes it look like the network failed and obscures
|
||||
the provider that needs attention. The diagnostic deliberately contains
|
||||
only response shape metadata: never response content or credentials.
|
||||
"""
|
||||
raw = response.read()
|
||||
content_type = ""
|
||||
headers = getattr(response, "headers", None)
|
||||
if headers is not None:
|
||||
try:
|
||||
content_type = str(headers.get("Content-Type", "")).split(";", 1)[0].lower()
|
||||
except (AttributeError, TypeError):
|
||||
content_type = ""
|
||||
if not raw:
|
||||
fail(f"provider_response_empty {provider} content_type={content_type or 'unknown'}")
|
||||
try:
|
||||
payload = json.loads(raw.decode("utf-8"))
|
||||
except (UnicodeDecodeError, json.JSONDecodeError) as exc:
|
||||
fail(
|
||||
f"provider_response_invalid {provider} {type(exc).__name__} "
|
||||
f"bytes={len(raw)} content_type={content_type or 'unknown'}"
|
||||
)
|
||||
if not isinstance(payload, dict):
|
||||
fail(f"provider_response_invalid {provider} payload_type={type(payload).__name__}")
|
||||
return payload
|
||||
|
||||
|
||||
def enforce_call_budget():
|
||||
"""Fail closed when a run exceeds CASAN_MODEL_MAX_CALLS. The count is tracked in
|
||||
CASAN_MODEL_CALL_COUNTER_FILE so it spans the many per-step model-call processes
|
||||
@@ -192,7 +223,7 @@ def call_ollama(model_name, prompt, role):
|
||||
t0 = time.time()
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=REQUEST_TIMEOUT) as resp:
|
||||
payload = json.loads(resp.read().decode())
|
||||
payload = decode_provider_json(resp, "ollama")
|
||||
except Exception as exc: # backend/model failure -> honest non-zero, no fake success
|
||||
fail(f"backend_unreachable {type(exc).__name__}: {str(exc)[:120]}")
|
||||
latency_ms = int((time.time() - t0) * 1000)
|
||||
@@ -226,7 +257,7 @@ def call_openai(model_name, prompt, role):
|
||||
t0 = time.time()
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=REQUEST_TIMEOUT) as resp:
|
||||
payload = json.loads(resp.read().decode())
|
||||
payload = decode_provider_json(resp, "openai-chat")
|
||||
except Exception as exc: # honest non-zero, no fake success
|
||||
fail(f"backend_unreachable {type(exc).__name__}: {str(exc)[:120]}")
|
||||
latency_ms = int((time.time() - t0) * 1000)
|
||||
@@ -278,7 +309,7 @@ def call_openai_responses(model_name, prompt, role):
|
||||
t0 = time.time()
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=REQUEST_TIMEOUT) as resp:
|
||||
payload = json.loads(resp.read().decode())
|
||||
payload = decode_provider_json(resp, "openai-responses")
|
||||
except Exception as exc:
|
||||
fail(f"backend_unreachable {type(exc).__name__}: {str(exc)[:120]}")
|
||||
latency_ms = int((time.time() - t0) * 1000)
|
||||
@@ -345,7 +376,7 @@ def call_openai_compatible(model_name, prompt, role):
|
||||
t0 = time.time()
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=REQUEST_TIMEOUT) as resp:
|
||||
payload = json.loads(resp.read().decode())
|
||||
payload = decode_provider_json(resp, "openai-compatible")
|
||||
except Exception as exc:
|
||||
fail(f"backend_unreachable {type(exc).__name__}: {str(exc)[:120]}")
|
||||
latency_ms = int((time.time() - t0) * 1000)
|
||||
@@ -413,7 +444,7 @@ def call_anthropic(model_name, prompt, role):
|
||||
t0 = time.time()
|
||||
try:
|
||||
with urllib.request.urlopen(req, timeout=REQUEST_TIMEOUT) as resp:
|
||||
payload = json.loads(resp.read().decode())
|
||||
payload = decode_provider_json(resp, "anthropic")
|
||||
except Exception as exc: # honest non-zero, no fake success
|
||||
fail(f"backend_unreachable {type(exc).__name__}: {str(exc)[:120]}")
|
||||
latency_ms = int((time.time() - t0) * 1000)
|
||||
|
||||
@@ -89,8 +89,26 @@ class GoalPatchWorkflowTests(unittest.TestCase):
|
||||
self.assertEqual(output, repaired)
|
||||
self.assertEqual(reason, "ok")
|
||||
self.assertEqual(usage["repair_model"], "openai-compatible:aug/claude-sonnet")
|
||||
self.assertEqual(usage["repair_attempts"], [
|
||||
{"attempt": 1, "provider": "openai", "model": "openai:gpt-4.1", "status": "failed", "reason": "goal_patch_check_failed:corrupt"},
|
||||
{"attempt": 2, "provider": "omniroute", "model": "openai-compatible:aug/claude-sonnet", "status": "pass", "reason": "ok"},
|
||||
])
|
||||
self.assertEqual([item.args[0] for item in call.call_args_list], ["openai:gpt-4.1", "openai-compatible:aug/claude-sonnet"])
|
||||
|
||||
def test_repair_records_each_provider_failure_without_hiding_the_primary(self):
|
||||
with (
|
||||
patch.dict(os.environ, {"CASAN_GOAL_PATCH_REPAIR_ATTEMPTS": "2", "CASAN_GOAL_PATCH_REPAIR_MODELS": "openai:gpt-5.3-codex,openai-compatible:gateway"}, clear=False),
|
||||
patch.object(ORCHESTRATOR, "call_model", side_effect=[
|
||||
(False, "", {}, "model_exit_2:MODEL_ROUTER_ERROR provider_response_empty openai-responses content_type=unknown"),
|
||||
(False, "", {}, "model_exit_2:MODEL_ROUTER_ERROR provider_response_invalid openai-compatible JSONDecodeError bytes=0 content_type=unknown"),
|
||||
]),
|
||||
):
|
||||
ok, _, usage, reason = ORCHESTRATOR.repair_write_output("ollama:ornith", "original", "plan")
|
||||
self.assertFalse(ok)
|
||||
self.assertIn("openai-compatible", reason)
|
||||
self.assertEqual([entry["provider"] for entry in usage["repair_attempts"]], ["openai", "omniroute"])
|
||||
self.assertIn("openai-responses", usage["repair_attempts"][0]["reason"])
|
||||
|
||||
def test_patch_outside_workspace_is_denied(self):
|
||||
job = {"workspace": {"context_roots": ["apps/okr/frontend"]}}
|
||||
content = "diff --git a/package.json b/package.json\n--- a/package.json\n+++ b/package.json\n@@ -1 +1 @@\n-a\n+b\n"
|
||||
|
||||
@@ -150,6 +150,15 @@ class FakeResp:
|
||||
def read(self):
|
||||
return json.dumps(self.payload).encode()
|
||||
|
||||
class EmptyResp:
|
||||
headers = {"Content-Type": "application/json"}
|
||||
def __enter__(self):
|
||||
return self
|
||||
def __exit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
def read(self):
|
||||
return b""
|
||||
|
||||
def fake_urlopen(req, timeout):
|
||||
seen.append((req.full_url, dict(req.header_items()), json.loads(req.data.decode())))
|
||||
if req.full_url.endswith("/v1/responses"):
|
||||
@@ -207,6 +216,14 @@ for fn, bad in (
|
||||
assert exc.code == 2, exc.code
|
||||
else:
|
||||
raise AssertionError(f"{fn.__name__} accepted malformed provider payload")
|
||||
|
||||
try:
|
||||
with contextlib.redirect_stderr(io.StringIO()):
|
||||
mc.decode_provider_json(EmptyResp(), "openai-compatible")
|
||||
except SystemExit as exc:
|
||||
assert exc.code == 2, exc.code
|
||||
else:
|
||||
raise AssertionError("empty provider response was accepted")
|
||||
print("ok")
|
||||
PY
|
||||
[[ $? -eq 0 ]] && pass "cloud/gateway provider responses parse real usage and reject malformed payloads" || fail "cloud/gateway provider parser coverage failed"
|
||||
|
||||
Reference in New Issue
Block a user