diff --git a/docs/reference/goal-acceptance-observations.md b/docs/reference/goal-acceptance-observations.md index 1e4e0bc4d6..41b9467201 100644 --- a/docs/reference/goal-acceptance-observations.md +++ b/docs/reference/goal-acceptance-observations.md @@ -98,6 +98,23 @@ for inline context, one-call writeback and upgrade/adoption limits. Goal 完成;要求/工作变化使依据过期,产物修改后须重新核对。既有可执行研究及 原始预算/停止条件保持权威。 +Native `goal-acceptance` and outcome-update `refresh-state` rejections retain +their stable error code. A known outcome validation rule also supplies optional +`error_detail`, such as `source ref must be an opaque identifier`, in JSON and +Markdown. This detail contains fixed rule text, never the rejected value or +provider/IO context. Validation rejection writes nothing: after confirming that +result, correct the document and retry its operation with the still-current +provider revision. An uncertain write still follows the original receipt and +recovery rules. Successful operations and unmarked failures keep their existing +behavior. + +中文:原生 `goal-acceptance` 和成果更新 `refresh-state` 拒绝保留稳定错误码。 +已知成果校验规则还会在 JSON 与 Markdown 中提供可选 `error_detail`,例如 +`source ref must be an opaque identifier`。详情只包含固定规则,不回显被拒值或 +provider/IO 上下文。校验拒绝不写入状态;确认这一结果后,修正文档并沿用原操作和 +仍有效的 provider revision 重试。写入结果不确定时继续遵循原有回执与恢复规则。 +成功操作和未标记错误保持既有行为。 + ## Executable checks within default acceptance diff --git a/loopx/cli_commands/goal_acceptance.py b/loopx/cli_commands/goal_acceptance.py index 2702576df6..f5c37ac4fc 100644 --- a/loopx/cli_commands/goal_acceptance.py +++ b/loopx/cli_commands/goal_acceptance.py @@ -10,6 +10,7 @@ from typing import Any from ..control_plane.goals.acceptance import ( + GoalAcceptanceRejected, configure_goal_acceptance, refine_goal_outcome, inspect_goal_acceptance, @@ -52,7 +53,10 @@ def register_goal_acceptance_command(subparsers: Any, add_format: Any) -> None: def render_goal_acceptance(payload: dict[str, Any]) -> str: lines = ["# Goal acceptance", "", f"- ok: {payload.get('ok')}"] if payload.get("error"): - return "\n".join([*lines, f"- error: {payload['error']}"]) + lines.append(f"- error: {payload['error']}") + if payload.get("error_detail"): + lines.append(f"- detail: {payload['error_detail']}") + return "\n".join(lines) if payload.get("provider_revision"): lines.append(f"- provider revision: {payload['provider_revision']}") contract = payload.get("goal_acceptance_contract") or {} @@ -166,6 +170,9 @@ def handle_goal_acceptance_command( else 0 ) except (OSError, TypeError, ValueError, RuntimeError) as exc: - payload, code = {"ok": False, "error": str(exc)}, 1 + payload, code = { + "ok": False, "error": str(exc), + **({"error_code": exc.code, **exc.payload} if isinstance(exc, GoalAcceptanceRejected) else {}), + }, 1 print_payload(payload, output_format(args), render_goal_acceptance) return code diff --git a/loopx/control_plane/goals/acceptance.py b/loopx/control_plane/goals/acceptance.py index c69761bccc..ad03aa9cad 100644 --- a/loopx/control_plane/goals/acceptance.py +++ b/loopx/control_plane/goals/acceptance.py @@ -32,6 +32,16 @@ _INSPECT_METHOD = "goal.acceptance.inspect" + +class GoalAcceptanceRejected(ValueError): + """Carry owner-qualified detail through the existing CLI error boundary.""" + + def __init__(self, code: str, detail: str) -> None: + super().__init__(code) + self.code = code + self.payload = {"error_detail": detail} + + def _routing( registry_path: Path, goal_id: str, @@ -76,13 +86,17 @@ def _result( "planned", "no_change", }: - raise ValueError( - str( - value.get("reason_code") - or value.get("reason") - or "Goal acceptance authority is unavailable" - ) + reason = str( + value.get("reason_code") + or value.get("reason") + or "Goal acceptance authority is unavailable" ) + detail = value.get("error_detail") + if isinstance(value.get("reason_code"), str) and isinstance(detail, str) and detail: + # This field is emitted only for typed, fixed-rule validation. + # Never promote the generic reason, which can contain private IO. + raise GoalAcceptanceRejected(reason, detail) + raise ValueError(reason) return value diff --git a/loopx/control_plane/goals/acceptance_authority.ts b/loopx/control_plane/goals/acceptance_authority.ts index 59df708335..56e205bbf7 100644 --- a/loopx/control_plane/goals/acceptance_authority.ts +++ b/loopx/control_plane/goals/acceptance_authority.ts @@ -1,4 +1,4 @@ -import {bindGoalOutcomesStateOwner, goalOutcomeCheckBasis, refineGoalOutcome} from "./outcome_requirements.ts"; +import {bindGoalOutcomesStateOwner, GoalOutcomeValidationError, goalOutcomeCheckBasis, refineGoalOutcome} from "./outcome_requirements.ts"; /** Acceptance effects use the existing canonical head, CAS and operation journal. * Only trusted local owner/host adapters may invoke these mutation exports. */ import {isAbsolute} from "node:path"; @@ -564,6 +564,7 @@ async function local(value: unknown, kind: "inspect" | "configure" | "verify" | ? error.message : null; const result = {...failure(taskReason ?? (error instanceof AuthorityStoreProtocolError ? "goal_acceptance_invalid_request" : "goal_acceptance_effect_failed"), error instanceof Error ? error.message : "acceptance effect failed"), + ...(error instanceof GoalOutcomeValidationError ? {error_detail: error.message} : {}), decision_read_from_provider: false, legacy_fallback_used: false, ...localAuthorityOpenFailure(error)}; return store ? {...result, source_authority: authorityStoreSourceAuthority(store)} : result; } diff --git a/loopx/control_plane/goals/outcome_requirements.ts b/loopx/control_plane/goals/outcome_requirements.ts index 46188008fe..b91de710ce 100644 --- a/loopx/control_plane/goals/outcome_requirements.ts +++ b/loopx/control_plane/goals/outcome_requirements.ts @@ -22,8 +22,12 @@ interface OutcomeState extends JsonObject { schema_version: typeof GOAL_OUTCOMES_SCHEMA; goal_id: string; goal_ref: WireExactGoalRef | null; revision: number; outcomes: Outcome[]; } +/** Only fixed rule text and validator-owned labels belong in public detail. + * Caller values and arbitrary provider/IO errors must never use this marker. */ +export class GoalOutcomeValidationError extends AuthorityStoreProtocolError {} + function requireValue(condition: unknown, reason: string): asserts condition { - if (!condition) throw new AuthorityStoreProtocolError(reason); + if (!condition) throw new GoalOutcomeValidationError(reason); } function text(value: unknown, label: string, max = 512): string { requireValue(typeof value === "string" && value === value.trim() && !!value && value.length <= max && !value.includes("\0"), `${label} must be bounded nonempty text`); diff --git a/loopx/state_refresh.py b/loopx/state_refresh.py index 5b94953f73..a8e2f54290 100644 --- a/loopx/state_refresh.py +++ b/loopx/state_refresh.py @@ -547,6 +547,8 @@ def render_state_refresh_markdown(payload: dict[str, Any]) -> str: ) if payload.get("error"): lines.append(f"- error: {payload.get('error')}") + if payload.get("error_detail"): + lines.append(f"- detail: {payload['error_detail']}") return "\n".join(lines) repair_delta = ( diff --git a/tests/control_plane/test_goal_outcome_diagnostics.py b/tests/control_plane/test_goal_outcome_diagnostics.py new file mode 100644 index 0000000000..0d087283c1 --- /dev/null +++ b/tests/control_plane/test_goal_outcome_diagnostics.py @@ -0,0 +1,107 @@ +"""Rejected refinements explain the rule without echoing caller values. + +The real CLI and canonical File/SQLite owner prove that rejection writes +nothing and that correcting the same operation commits exactly once. +""" +from __future__ import annotations + +import json +import subprocess +import sys + +import pytest + +from test_goal_acceptance_cli import acceptance_goal as acceptance_goal + + +def refinement(source_ref): + return { + "outcome_id": "export", + "owner_agent_id": "agent-a", + "objective": "Produce a usable export", + "requirements": [{"id": "content", "description": "Retain the content"}], + "source": {"ref": source_ref, "summary": "Synthetic source summary"}, + } + + +def arguments(project, cli, entrypoint, document, operation): + revision = cli("inspect")[1]["provider_revision"] + path = project / "refinement.json" + if entrypoint == "refine": + path.write_text(json.dumps(document)) + return path, ("goal-acceptance", "refine", "--goal-id", "goal-acceptance", + "--agent-id", "agent-a", "--document", str(path), "--operation-id", operation, + "--expected-provider-revision", revision, "--execute") + path.write_text(json.dumps({"refinement": document, "operation_id": operation, + "expected_provider_revision": revision})) + return path, ("refresh-state", "--goal-id", "goal-acceptance", "--agent-id", "agent-a", + "--no-global-sync", "--goal-outcome-update-json", str(path)) + + +@pytest.mark.parametrize("entrypoint", ["refine", "refresh"]) +@pytest.mark.parametrize(("source_ref", "detail"), [ + ("synthetic/private-ref", "source ref must be an opaque identifier"), + ("synthetic-private-ref ", "source ref must be bounded nonempty text"), +]) +def test_native_rejection_then_corrected_same_operation(acceptance_goal, entrypoint, source_ref, detail): + project, _, cli, run = acceptance_goal + before = cli("inspect")[1] + state_before = (project / "state.md").read_bytes() + path, args = arguments(project, cli, entrypoint, refinement(source_ref), "correctable-operation") + code, rejected = run(*args) + assert code == 1 and rejected["error"] == "goal_acceptance_invalid_request", rejected + assert rejected["error_code"] == "goal_acceptance_invalid_request" + assert rejected["error_detail"] == detail + assert source_ref not in json.dumps(rejected) + assert "Synthetic source summary" not in json.dumps(rejected) + assert cli("inspect")[1] == before + assert (project / "state.md").read_bytes() == state_before + if entrypoint == "refresh": + assert rejected["appended"] is False + + document = json.loads(path.read_text()) + patch = document if entrypoint == "refine" else document["refinement"] + patch["source"]["ref"] = "evidence:export-v1" + path.write_text(json.dumps(document)) + code, applied = run(*args) + assert code == 0, applied + result = applied if entrypoint == "refine" else applied["goal_outcome_update"] + assert result["status"] == "applied" + assert "error_detail" not in applied and "error_detail" not in result + accepted = cli("inspect")[1] + assert accepted["provider_revision"] != before["provider_revision"] + code, repeated = run(*args) + assert code == 0, repeated + replay = repeated if entrypoint == "refine" else repeated["goal_outcome_update"] + assert replay["status"] == "replayed" + assert cli("inspect")[1] == accepted + + +@pytest.mark.parametrize("entrypoint", ["refine", "refresh"]) +def test_native_markdown_shows_rule_without_the_rejected_value(acceptance_goal, entrypoint): + project, _, cli, _ = acceptance_goal + source_ref = "synthetic/private-markdown-value" + _, args = arguments(project, cli, entrypoint, refinement(source_ref), "markdown-operation") + result = subprocess.run([sys.executable, "-m", "loopx.cli", + "--registry", str(project.parent / "registry.json"), + "--runtime-root", str(project.parent / "runtime"), *args], + capture_output=True, text=True, timeout=60) + assert result.returncode == 1, result.stdout + result.stderr + assert "goal_acceptance_invalid_request" in result.stdout + assert "source ref must be an opaque identifier" in result.stdout + assert source_ref not in result.stdout + result.stderr + + +def test_unqualified_effect_reason_is_not_promoted_to_public_detail(monkeypatch): + from loopx.control_plane.goals import acceptance + + private_reason = "synthetic-private-provider-context" + monkeypatch.setattr(acceptance, "effect_runtime_result", lambda *_: { + "status": "failed", "reason_code": "goal_acceptance_effect_failed", + "reason": private_reason, + }) + with pytest.raises(ValueError) as failure: + acceptance._result("goal.acceptance.refine", {}) + assert str(failure.value) == "goal_acceptance_effect_failed" + assert private_reason not in str(failure.value) + assert not getattr(failure.value, "payload", {}).get("error_detail")