diff --git a/docs/reference/goal-acceptance-observations.md b/docs/reference/goal-acceptance-observations.md index d81be96c4f..baffb28d95 100644 --- a/docs/reference/goal-acceptance-observations.md +++ b/docs/reference/goal-acceptance-observations.md @@ -131,6 +131,22 @@ reconciled. Readback never runs checks. A stored successful receipt cannot certify changed files, independent reviewer identity, adoption or Goal completion. A later finished/cancelled plan does not resurrect an older draft. +Prepared plans bind the reviewed acceptance configuration and declared work, +rather than every update to the Goal's provider revision. Notes and work outside +an explicit `selected_work` scope do not require preparing the same plan again. +Changed covered requirements, a new owner configuration or another Goal instance +still invalidate it. TypeScript classifies that context; each actual write keeps +the current provider CAS. The original operation and configuration context remain +bound to native receipts, including recovery after a lost response. Historical +plans without a reviewed context retain their original strict revision check. + +Retrying preparation with the same operation, Goal, preparing Agent and document +returns the original saved proposal, even after unrelated progress or an owner +decision. It does not refresh the reviewed basis, rerun checks or reopen a +cancelled, stale or applied plan. Changed intent still conflicts, and applying +the recovered proposal still validates the current acceptance context. Its +historical receipt is not current acceptance. + Count the complete task: with an Agent-prepared plan already delivered, the owner has one required decision. If the original request still lacks acceptance intent or authority, that missing decision is an additional round; preparing a proposal @@ -146,9 +162,15 @@ No-plan/default-off behavior remains unchanged, and no Lark action is introduced Goal、Agent 与运行目录由宿主固定,暂停、过期 Turn 或撤销配置后不能继续准备。 旧原生线程的工具声明不会自动升级,也不会为新增工具替换未完成线程;未声明时保留 CLI 准备路径。该入口不自动启用 LoopX、不配置或运行检查。 +准备响应丢失后,以相同操作、Goal、准备 Agent 和文档重试会读回原方案及其当前记录状态, +不因进展或所有者决定而刷新审阅依据,也不重跑检查或重新打开已取消、过期、已采用的方案。 +意图改变仍报冲突;执行原方案仍检查当前依据,历史回执不代表当前验收。 概览将同一个交付检查交互放在“方向”正文中,先于工作和成果列表;目标、条件和当前结果 使用同一验收读回。桌面展开检查细节,窄屏默认折叠条件、版本记录与高级迁移入口,但不折叠 必需动作、命令影响或失败反馈。已有配置的重查使用次要按钮,待采用方案保留明确的采用决定。 +待采用方案绑定实际审阅的验收规则与工作声明;备注或明确范围外的工作变化,不要求重复准备。 +覆盖任务要求、所有者配置或 Goal 实例变化仍使方案失效。TypeScript 判断依据,写入继续使用 +当前 provider CAS;原操作与配置依据由原生回执保留。旧方案维持原来的严格版本检查。 For advanced CLI configuration, inspect the current provider revision: diff --git a/examples/personal-workspace-browser/goal-acceptance-oracle-backend.py b/examples/personal-workspace-browser/goal-acceptance-oracle-backend.py index e49372a31f..7690d8ac59 100644 --- a/examples/personal-workspace-browser/goal-acceptance-oracle-backend.py +++ b/examples/personal-workspace-browser/goal-acceptance-oracle-backend.py @@ -16,7 +16,7 @@ from loopx.control_plane.coordination.runtime_shadow import ( build_todo_runtime_shadow_projection, ) -from loopx.control_plane.testing.canary_harness import write_fixture_registry +from loopx.control_plane.testing.canary_harness import run_json_cli_result, write_fixture_registry sys.path.insert(0, str(Path(__file__).resolve().parents[2] / "tests" / "control_plane")) from canonical_authority_fixture import initialize_canonical_authority @@ -127,6 +127,17 @@ "action": "prepare_check", "operation_id": operation, "document": document}) if not prepared.get("ok"): raise RuntimeError(f"Conversation preparation failed: {prepared}") +# Simulate a lost tool reply followed by ordinary progress. The original plan +# must reach the existing owner action, rather than a newly prepared approval. +code, progress = run_json_cli_result("todo", "update", "--goal-id", "multi-agent-projection", + "--todo-id", "todo_work", "--agent-id", "agent-a", "--note", "Work is progressing", + registry_path=registry, runtime_root=runtime) +if code != 0: + raise RuntimeError(f"Fixture progress failed: {progress}") +recovered = adapter.session.read_tool_handler(TOOL["name"], { + "action": "prepare_check", "operation_id": "prepare-export-check", "document": document}) +if recovered != prepared: + raise RuntimeError(f"The original check plan was not recovered: {recovered}") server = ChatHTTPServer(("127.0.0.1", 0), ChatRequestHandler) server.registry_path, server.runtime_root = registry, runtime server.runtime_root_override, server.verbose = str(runtime), False diff --git a/loopx/control_plane/goals/acceptance.py b/loopx/control_plane/goals/acceptance.py index 98d6c78898..1d5d653e24 100644 --- a/loopx/control_plane/goals/acceptance.py +++ b/loopx/control_plane/goals/acceptance.py @@ -233,6 +233,7 @@ def configure_goal_acceptance( operation_id: str | None = None, execute: bool = False, disable: bool = False, + review_context_digest: str | None = None, ) -> dict[str, Any]: """Explicit local-owner configuration; Agent tool callers have no writer.""" route = _routing(registry_path, goal_id, runtime_root, agent_id) @@ -246,6 +247,7 @@ def configure_goal_acceptance( "disable": disable, "dry_run": not execute, "operation_id": operation_id or f"goal-acceptance:{uuid4().hex}", + **({"review_context_digest": review_context_digest} if review_context_digest is not None else {}), }, ) diff --git a/loopx/control_plane/goals/acceptance_actions.py b/loopx/control_plane/goals/acceptance_actions.py index 699bbeb674..8bdeebd3fc 100644 --- a/loopx/control_plane/goals/acceptance_actions.py +++ b/loopx/control_plane/goals/acceptance_actions.py @@ -7,6 +7,7 @@ from __future__ import annotations +import json from pathlib import Path from typing import Any, TypedDict from uuid import uuid4 @@ -25,6 +26,32 @@ class _AcceptanceRoute(TypedDict): goal_id: str +def _review_basis(fingerprint: str) -> dict[str, str]: + # Historical proposals stored only a provider revision. New fingerprints + # carry that original CAS plus the opaque context digest made by TS. + if not fingerprint.startswith("{"): + return {"provider_revision": fingerprint} + value = json.loads(fingerprint) + if (not isinstance(value, dict) or set(value) != {"provider_revision", "review_context_digest"} + or any(not isinstance(item, str) or not item for item in value.values())): + raise ValueError("The saved acceptance review basis is invalid") + return value + + +def _saved_check(store: ChatActionStore, parameters: dict[str, Any], + operation_id: str) -> dict[str, Any] | None: + """Recover a journaled intent, never refresh its basis or lifecycle.""" + for proposal in store.list(goal_id=parameters["goal_id"], context_kind="goal"): + if proposal["idempotency_key"] != operation_id: + continue + if (proposal["action_kind"] != "goal.acceptance.check" + or proposal["context"] != {"kind": "goal", "goal_id": parameters["goal_id"]} + or proposal["normalized_parameters"] != parameters): + raise ActionConflictError("The operation belongs to another check request") + return proposal + return None + + def prepare_acceptance_check(*, registry_path: Path, runtime_root: str | None, goal_id: str, agent_id: str | None, document: dict[str, Any], operation_id: str, @@ -46,10 +73,26 @@ def prepare_acceptance_check(*, registry_path: Path, runtime_root: str | None, registry_path=registry_path) parameters = service._normalize("goal.acceptance.check", { "goal_id": goal_id, "document": document, "prepared_by": agent_id}) - proposal = service.preview({"action_kind": "goal.acceptance.check", - "summary": str(document.get("objective") or "Check the delivery"), - "idempotency_key": operation_id, "context": {"kind": "goal", "goal_id": goal_id}, - "normalized_parameters": parameters}) + proposal = _saved_check(service.store, parameters, operation_id) + recovered = proposal is not None + if proposal is None: + try: + proposal = service.preview({"action_kind": "goal.acceptance.check", + "summary": str(document.get("objective") or "Check the delivery"), + "idempotency_key": operation_id, "context": {"kind": "goal", "goal_id": goal_id}, + "normalized_parameters": parameters}) + except ActionConflictError: + # Another caller may have persisted this intent while the current + # caller read a newer basis. Keep the winner's immutable preview. + proposal = _saved_check(service.store, parameters, operation_id) + if proposal is None: + raise + recovered = True + if recovered: + # A historical proposal cannot stand in for an unavailable or retired + # canonical Goal. Reading current authority does not refresh its basis. + inspect_goal_acceptance(registry_path=registry_path, runtime_root=str(root), + goal_id=goal_id, agent_id=agent_id) return {"ok": True, "goal_id": goal_id, "prepared": True, "executed": False, "proposal": proposal} @@ -73,7 +116,9 @@ def _acceptance_preview(self, parameters: dict[str, Any]) -> tuple[str, dict[str ) if preview.get("status") != "planned": raise ValueError("The current Goal cannot prepare this check plan") - return before["provider_revision"], preview + fingerprint = json.dumps({"provider_revision": before["provider_revision"], + "review_context_digest": preview["review_context_digest"]}, sort_keys=True, separators=(",", ":")) + return fingerprint, preview def _apply_acceptance_check(self, proposal_id: str) -> dict[str, Any]: # The existing generic start_apply is replayable. Serialize this host @@ -91,13 +136,15 @@ def _apply_acceptance_check(self, proposal_id: str) -> dict[str, Any]: configured = steps.get("acceptance_configured") operation_id = f"acceptance-check:{proposal_id}" if configured is None: - expected = proposal["expected_state_fingerprint"] + basis = _review_basis(proposal["expected_state_fingerprint"]) + expected = basis["provider_revision"] try: # Its native idempotency receipt also recovers a lost # configuration response before the Chat checkpoint. saved = configure_goal_acceptance( **route, document=parameters["document"], expected_provider_revision=expected, + review_context_digest=basis.get("review_context_digest"), operation_id=f"acceptance-configure:{proposal_id}", execute=True, ) except ValueError: @@ -108,7 +155,9 @@ def _apply_acceptance_check(self, proposal_id: str) -> dict[str, Any]: return {"proposal": stored, "turn": None} raise configured = {"provider_revision": saved["provider_revision"], - "digest": saved["goal_acceptance_contract"]["digest"]} + "digest": saved["goal_acceptance_contract"]["digest"], + **({"review_context_digest": saved["review_context_digest"]} + if "review_context_digest" in saved else {})} self.store.save_checkpoint(proposal_id, step="acceptance_configured", receipt=configured) current = inspect_goal_acceptance(**route) contract = current["goal_acceptance_contract"] @@ -121,13 +170,18 @@ def _apply_acceptance_check(self, proposal_id: str) -> dict[str, Any]: result = {**public_goal_acceptance(current), "executed": True, "checks_passed": all(row["passed"] for row in observed["results"])} else: - if (current["provider_revision"] != configured["provider_revision"] - or contract.get("digest") != configured["digest"]): + expected = configured["provider_revision"] + if "review_context_digest" in configured: + fingerprint, preview = self._acceptance_preview(parameters) + if _review_basis(fingerprint).get("review_context_digest") != configured["review_context_digest"]: + raise ActionConflictError("The check basis changed. Prepare a current plan before executing.") + expected = preview["provider_revision"] + elif (current["provider_revision"] != expected or contract.get("digest") != configured["digest"]): raise ActionConflictError("The check basis changed. Prepare a current plan before executing.") self.store.save_checkpoint(proposal_id, step="acceptance_check_started", receipt={"operation_id": operation_id}) result = verify_goal_acceptance(**route, execute=True, - expected_provider_revision=configured["provider_revision"], operation_id=operation_id) + expected_provider_revision=expected, operation_id=operation_id) receipt = {"receipt_id": f"acceptance-{proposal_id}", "outcome": "acceptance_checked", "projection_verified": True, "goal_id": parameters["goal_id"], "operation_id": operation_id, "checks_passed": result["checks_passed"]} diff --git a/loopx/control_plane/goals/acceptance_authority.ts b/loopx/control_plane/goals/acceptance_authority.ts index 32a360cef9..b44c503840 100644 --- a/loopx/control_plane/goals/acceptance_authority.ts +++ b/loopx/control_plane/goals/acceptance_authority.ts @@ -13,7 +13,7 @@ import {GOAL_ACCEPTANCE_OWNED_SCHEMA, GOAL_ACCEPTANCE_SCHEMA, acceptanceKeys, ac acceptanceText, acceptanceTodos, acceptanceCompletionRequirements, bindGoalAcceptanceStateOwner, goalAcceptanceTodoDigest, goalAcceptanceWorkDigest, normalizeAcceptanceResults, normalizeGoalAcceptanceDocument, projectGoalAcceptance, readGoalAcceptance, readGoalAcceptanceAuthority, - type AcceptanceState, type AcceptanceVerification} from "./acceptance_contract.ts"; + type AcceptanceDocument, type AcceptanceState, type AcceptanceVerification} from "./acceptance_contract.ts"; import { BARE_SHA256_PATTERN } from "../content_digest.ts"; import {GOAL_ACCEPTANCE_LIFECYCLE_SCHEMA, parseGoalAcceptanceLifecycleTransition, parseWireExactGoalRef, readGoalAcceptanceLifecycle, sameExactGoalRef, type GoalAcceptanceLifecycle, @@ -33,7 +33,7 @@ function mutationRequest(value: unknown, verification: boolean): JsonObject { // its wire call need not impersonate an owner. Explicit Agent actors fail. if (verification && !Object.hasOwn(request, "actor_agent_id")) request.actor_agent_id = null; acceptanceKeys(request, [...REQUEST_FIELDS, ...(verification ? ["contract_digest", "revision", "results"] : ["document"])], - [...LOCAL_FIELDS, "goal_ref", ...(verification ? ["todo_id"] : ["disable"])]); + [...LOCAL_FIELDS, "goal_ref", ...(verification ? ["todo_id"] : ["disable", "review_context_digest"])]); acceptanceRequire(request.actor_agent_id === null, "goal acceptance mutation requires the trusted owner/host; agent actors cannot configure or attest acceptance"); for (const field of ["goal_id", "operation_id", "expected_provider_revision"]) { requireAuthorityStoreId(request[field], field); @@ -41,6 +41,10 @@ function mutationRequest(value: unknown, verification: boolean): JsonObject { acceptanceText(request.operation_id, "acceptance operation id", 256); acceptanceRequire(request.dry_run === undefined || typeof request.dry_run === "boolean", "dry_run must be boolean"); if (!verification) acceptanceRequire(request.disable === undefined || typeof request.disable === "boolean", "disable must be boolean"); + if (request.review_context_digest !== undefined) acceptanceRequire( + typeof request.review_context_digest === "string" && BARE_SHA256_PATTERN.test(request.review_context_digest) + && request.document !== null && request.disable !== true, + "review context requires a document and its typed acceptance digest"); if (request.goal_ref !== undefined) parseWireExactGoalRef(request.goal_ref, "acceptance request goal_ref"); return request; } @@ -98,6 +102,15 @@ function current(loaded: AuthorityStoreLoadResult, request: JsonObject): Authori function loaded(value: AuthorityStoreHead | JsonObject): value is AuthorityStoreHead { return "status" in value && value.status === "loaded"; } +/** Only this owner classifies the work and governance an owner reviewed. + * Verification observations and unrelated provider writes cannot change it. */ +function reviewContext(head: JsonObject, goalId: string, document: AcceptanceDocument): string { + const state = readGoalAcceptance(head, goalId); + return canonicalAuthoritySha256({goal_id: goalId, document, + lifecycle: readGoalAcceptanceLifecycle(head, goalId), + contract: state === null ? null : {...state, verification: null}, + work_digest: goalAcceptanceWorkDigest(head, goalId, document.scope)}); +} async function replayWithCurrentProjection( store: AuthorityStore, result: JsonObject, @@ -133,12 +146,20 @@ async function commit(store: AuthorityStore, request: JsonObject, head: Authorit }; } if (committedState !== null) next.goal_acceptance = committedState; + const reviewed = kind === "configure" && request.document !== null && committedState !== null; const result = {goal_id: request.goal_id, operation_id: request.operation_id, - goal_acceptance_contract: projectGoalAcceptance(next, String(request.goal_id))}; + goal_acceptance_contract: projectGoalAcceptance(next, String(request.goal_id)), + ...(reviewed && request.review_context_digest !== undefined ? { + review_context_digest: reviewContext(next, String(request.goal_id), committedState.document), + } : {})}; if (request.dry_run === true) return source(store, {status: "planned", dry_run: true, changed: false, - provider_revision: head.provider_revision, ...result}); + provider_revision: head.provider_revision, ...result, + ...(reviewed ? {review_context_digest: reviewContext(head.head, String(request.goal_id), committedState.document)} : {})}); const committed = await command.receipt.commit(store, { - operation_id: String(request.operation_id), expected_provider_revision: String(request.expected_provider_revision), + // Reviewed-context admission may accept an unrelated intervening write. + // CAS still compares the exact head just observed; receipt identity retains + // the original request, including its original revision and review digest. + operation_id: String(request.operation_id), expected_provider_revision: head.provider_revision, next_projection: next, events: [{schema_version: RECEIPT_SCHEMA, kind: `goal_acceptance_${kind}`, goal_id: request.goal_id, operation_id: request.operation_id, revision: committedState?.revision ?? null, @@ -161,7 +182,16 @@ export async function configureGoalAcceptance(store: AuthorityStore, value: Json if (observation.kind === "receipt") { return replayWithCurrentProjection(store, observation.result, String(request.goal_id)); } - const head = current(observation.authority, request); + let admission = request; + if (request.review_context_digest !== undefined && observation.authority.status === "loaded") { + acceptanceTodos(observation.authority.head, String(request.goal_id)); + if (document === null || reviewContext(observation.authority.head, String(request.goal_id), document) + !== request.review_context_digest) return source(store, { + ...failure("goal_acceptance_review_context_mismatch", "The reviewed work or acceptance configuration changed."), + status: "conflict", current_provider_revision: observation.authority.provider_revision}); + admission = {...request, expected_provider_revision: observation.authority.provider_revision}; + } + const head = current(observation.authority, admission); if (!loaded(head)) return source(store, head); const previous = readGoalAcceptance(head.head, String(request.goal_id)); let state: AcceptanceState | null = previous; diff --git a/tests/control_plane/test_goal_acceptance_prepared_action.py b/tests/control_plane/test_goal_acceptance_prepared_action.py index f0db2d513d..d3b4a4540a 100644 --- a/tests/control_plane/test_goal_acceptance_prepared_action.py +++ b/tests/control_plane/test_goal_acceptance_prepared_action.py @@ -10,6 +10,7 @@ from loopx.chat_actions import ChatActionService from loopx.chat_action_store import ActionConflictError, ChatActionStore from loopx.control_plane.goals import acceptance_actions +from loopx.control_plane.testing.canary_harness import run_json_cli_result from test_goal_acceptance_cli import acceptance_goal as acceptance_goal @@ -154,6 +155,96 @@ def test_conversation_check_returns_document_repair_context(conversation_check): assert not (project / "runs.txt").exists() +@pytest.mark.parametrize("state", ["preview_ready", "applied", "cancelled"]) +def test_preparation_retry_recovers_original_state_after_progress(conversation_check, state): + project, cli, controller, _, _, call, _ = conversation_check + original = call()["proposal"] # The reply may be lost after persistence. + store = ChatActionStore(project.parent / "runtime/chat/actions") + service = ChatActionService(store=store, registry_path=controller.registry_path) + if state == "applied": + original = service.apply(original["proposal_id"])["proposal"] + elif state == "cancelled": + original = service.cancel(original["proposal_id"]) + code, updated = run_json_cli_result("todo", "update", "--goal-id", "goal-acceptance", + "--todo-id", "todo_export", "--agent-id", "agent-a", "--note", "Work is progressing", + registry_path=controller.registry_path, runtime_root=project.parent / "runtime") + assert code == 0, updated + authority, journal = cli("inspect")[1], store.path.read_bytes() + recovered = call() + assert recovered["ok"] is True and recovered["executed"] is False, recovered + assert recovered["proposal"] == original + assert cli("inspect")[1] == authority and store.path.read_bytes() == journal + if state == "applied": + assert (project / "runs.txt").read_text() == "x" + else: + assert not (project / "runs.txt").exists() + if state == "preview_ready": + applied = service.apply(original["proposal_id"])["proposal"] + assert call()["proposal"] == applied + assert service.apply(original["proposal_id"])["proposal"] == applied + assert (project / "runs.txt").read_text() == "x" + + +def test_preparation_retry_does_not_return_another_preparers_plan(conversation_check): + project, cli, controller, _, plan, call, _ = conversation_check + original = call()["proposal"] + store = ChatActionStore(project.parent / "runtime/chat/actions") + authority, journal = cli("inspect")[1], store.path.read_bytes() + with pytest.raises(ActionConflictError): + acceptance_actions.prepare_acceptance_check(registry_path=controller.registry_path, + runtime_root=str(project.parent / "runtime"), goal_id="goal-acceptance", + agent_id="agent-a", document=plan, operation_id=original["idempotency_key"], execute=True) + assert cli("inspect")[1] == authority and store.path.read_bytes() == journal + assert not (project / "runs.txt").exists() + + +def test_preparation_retry_does_not_replace_unavailable_authority(conversation_check, monkeypatch): + project, cli, _, _, _, call, _ = conversation_check + call() + store = ChatActionStore(project.parent / "runtime/chat/actions") + authority, journal = cli("inspect")[1], store.path.read_bytes() + + def unavailable(**_kwargs): + raise ValueError("Goal acceptance authority is unavailable") + + monkeypatch.setattr(acceptance_actions, "inspect_goal_acceptance", unavailable) + assert call()["error"] == "collaboration_request_rejected" + assert cli("inspect")[1] == authority and store.path.read_bytes() == journal + assert not (project / "runs.txt").exists() + + +def test_prepare_recovers_competing_commit_with_a_newer_basis(conversation_check, monkeypatch): + project, cli, controller, _, _, call, _ = conversation_check + entered, proceed, requests = Event(), Event(), [] + native_preview = ChatActionService.preview + + def held_preview(service, request): + requests.append(request) + entered.set() + assert proceed.wait(20) + return native_preview(service, request) + + monkeypatch.setattr(ChatActionService, "preview", held_preview) + store = ChatActionStore(project.parent / "runtime/chat/actions") + service = ChatActionService(store=store, registry_path=controller.registry_path) + with ThreadPoolExecutor(max_workers=1) as pool: + pending = pool.submit(call) + try: + assert entered.wait(20) + winner = native_preview(service, requests[0]) + code, updated = run_json_cli_result("todo", "update", "--goal-id", "goal-acceptance", + "--todo-id", "todo_export", "--agent-id", "agent-a", "--note", "Concurrent progress", + registry_path=controller.registry_path, runtime_root=project.parent / "runtime") + assert code == 0, updated + authority, journal = cli("inspect")[1], store.path.read_bytes() + finally: + proceed.set() + recovered = pending.result(timeout=20) + assert recovered["proposal"] == winner, recovered + assert cli("inspect")[1] == authority and store.path.read_bytes() == journal + assert not (project / "runs.txt").exists() + + def prepare(acceptance_goal): project, document, cli, _ = acceptance_goal raw = json.loads(document.read_text()) @@ -175,6 +266,97 @@ def prepare(acceptance_goal): return project, cli, service, result["proposal"] +@pytest.mark.parametrize("change", ["selected_note", "unselected_work"]) +def test_reviewed_selected_plan_survives_unrelated_progress(acceptance_goal, change): + project, document, cli, run = acceptance_goal + raw = json.loads(document.read_text()) + raw["scope"] = {"kind": "selected_work", "todo_ids": ["todo_export"]} + document.write_text(json.dumps(raw)) + _, _, service, proposal = prepare(acceptance_goal) + if change == "selected_note": + code, result = run("todo", "update", "--goal-id", "goal-acceptance", + "--todo-id", "todo_export", "--agent-id", "agent-a", "--note", "Work is progressing") + else: + code, result = run("todo", "add", "--goal-id", "goal-acceptance", "--role", "agent", + "--text", "Independent work", "--action-kind", "implement", "--claimed-by", "agent-a") + assert code == 0, result + assert cli("inspect")[1]["goal_acceptance_contract"] == {"enabled": False} + assert not (project / "runs.txt").exists() + result = service.apply(proposal["proposal_id"]) + assert result["proposal"]["status"] == "applied" + assert (project / "runs.txt").read_text() == "x" + assert service.apply(proposal["proposal_id"])["proposal"] == result["proposal"] + assert (project / "runs.txt").read_text() == "x" + + +def test_historical_revision_only_plan_keeps_strict_admission(acceptance_goal): + project, _, _, run = acceptance_goal + _, _, service, proposal = prepare(acceptance_goal) + historical = service.store.create_preview( + **{key: proposal[key] for key in ( + "action_kind", "summary", "normalized_parameters", "context", + "permission_classification", "validation_evidence", "available_transitions")}, + expected_state_fingerprint=json.loads(proposal["expected_state_fingerprint"])["provider_revision"], + idempotency_key="historical-revision-only-plan", + ) + code, result = run("todo", "update", "--goal-id", "goal-acceptance", "--todo-id", "todo_export", + "--agent-id", "agent-a", "--note", "Progress after a historical plan") + assert code == 0, result + assert service.apply(historical["proposal_id"])["proposal"]["status"] == "stale" + assert not (project / "runs.txt").exists() + + +def test_reviewed_plan_rejects_changed_covered_work(acceptance_goal): + project, document, cli, run = acceptance_goal + _, _, service, proposal = prepare(acceptance_goal) + code, changed = run("todo", "update", "--goal-id", "goal-acceptance", "--todo-id", "todo_export", + "--agent-id", "agent-a", "--text", "Deliver a different export") + assert code == 0, changed + code, recovered = cli("prepare", "--agent-id", "agent-a", "--document", str(document), + "--operation-id", "prepare-check", "--execute") + assert code == 0 and recovered["proposal"] == proposal + assert service.apply(proposal["proposal_id"])["proposal"]["status"] == "stale" + code, recovered = cli("prepare", "--agent-id", "agent-a", "--document", str(document), + "--operation-id", "prepare-check", "--execute") + assert code == 0 and recovered["proposal"]["status"] == "stale" + assert not (project / "runs.txt").exists() + + +@pytest.mark.parametrize("changed", [False, True]) +def test_configuration_response_loss_keeps_its_original_review_basis(acceptance_goal, monkeypatch, changed): + project, document, cli, run = acceptance_goal + _, _, service, proposal = prepare(acceptance_goal) + original = service.store.save_checkpoint + + def lost(*args, **kwargs): + if kwargs["step"] == "acceptance_configured": + raise RuntimeError("lost configured checkpoint") + return original(*args, **kwargs) + + monkeypatch.setattr(service.store, "save_checkpoint", lost) + with pytest.raises(RuntimeError, match="lost"): + service.apply(proposal["proposal_id"]) + monkeypatch.setattr(service.store, "save_checkpoint", original) + if changed: + before = cli("inspect")[1] + code, result = cli("configure", "--document", str(document), + "--expected-provider-revision", before["provider_revision"], + "--operation-id", "new-owner-configuration", "--execute") + else: + code, result = run("todo", "update", "--goal-id", "goal-acceptance", "--todo-id", "todo_export", + "--agent-id", "agent-a", "--note", "Progress after the response was lost") + assert code == 0, result + if changed: + before = cli("inspect")[1] + with pytest.raises(ActionConflictError, match="basis changed"): + service.apply(proposal["proposal_id"]) + assert cli("inspect")[1] == before + assert not (project / "runs.txt").exists() + else: + assert service.apply(proposal["proposal_id"])["proposal"]["status"] == "applied" + assert (project / "runs.txt").read_text() == "x" + + def test_agent_preparation_does_not_approve_or_execute(acceptance_goal): project, cli, service, proposal = prepare(acceptance_goal) assert cli("inspect")[1]["goal_acceptance_contract"] == {"enabled": False} @@ -245,6 +427,9 @@ def test_stale_prepared_plan_cannot_replace_new_owner_configuration(acceptance_g "--expected-provider-revision", before["provider_revision"], "--operation-id", "another-owner-change", "--execute") assert code == 0, configured + code, recovered = cli("prepare", "--agent-id", "agent-a", "--document", str(document), + "--operation-id", "prepare-check", "--execute") + assert code == 0 and recovered["proposal"] == proposal result = service.apply(proposal["proposal_id"]) assert result["proposal"]["status"] == "stale" assert not (project / "runs.txt").exists() diff --git a/tests/control_plane_ts/goal_acceptance_authority.test.ts b/tests/control_plane_ts/goal_acceptance_authority.test.ts index 28501f092c..6b126c2e8f 100644 --- a/tests/control_plane_ts/goal_acceptance_authority.test.ts +++ b/tests/control_plane_ts/goal_acceptance_authority.test.ts @@ -233,6 +233,66 @@ async function fixture(t: TestContext, provider: typeof providers[number]): Prom for (const provider of providers) { const options = {skip: (provider === "postgresql" && !process.env.LOOPX_TEST_POSTGRES_URL) || (provider === "sqlite" && !sqliteQualified)}; + test(`${provider}: a scoped reviewed plan survives unrelated progress with exact native replay`, options, async t => { + const store = await fixture(t, provider); await seed(store); + const doc = {...document(), scope: {kind: "selected_work", todo_ids: ["todo_first"]}, + bindings: [{todo_id: "todo_first", criterion_ids: ["prerequisite"]}]}; + const request = await configureRequest(store, {document: doc}); + const before = await head(store); + const preview = await configureGoalAcceptance(store, {...request, dry_run: true}); + assert.deepEqual(await head(store), before); + assert.equal(typeof preview.review_context_digest, "string"); + await update(store, "todo_second", {text: "Changed independent work"}); + await update(store, "todo_first", {note: "Observed progress"}); + const stable = await head(store); + assert.equal((await configureGoalAcceptance(store, request)).reason_code, "goal_acceptance_provider_revision_mismatch", + "without explicit reviewed context, original CAS remains strict"); + assert.deepEqual(await head(store), stable); + const reviewed = {...request, review_context_digest: preview.review_context_digest}; + const applied = await configureGoalAcceptance(store, reviewed); + assert.equal(applied.status, "applied"); + const committed = await head(store); + await update(store, "todo_second", {note: "Later unrelated observation"}); + const replay = await configureGoalAcceptance(store, reviewed); + assert.equal(replay.status, "replayed"); + assert.equal(replay.review_context_digest, applied.review_context_digest, "receipt retains the original configured context"); + assert.notEqual((await head(store)).provider_revision, committed.provider_revision); + assert.equal((await configureGoalAcceptance(store, {...reviewed, + document: {...doc, objective: "Different approved effect"}})).reason_code, "coordination_operation_identity_mismatch"); + await assert.rejects(() => configureGoalAcceptance(store, {...reviewed, operation_id: randomUUID(), actor_agent_id: "worker"})); + }); + test(`${provider}: reviewed context retains exact CAS against a competing write`, options, async t => { + const store = await fixture(t, provider); await seed(store); + const request = await configureRequest(store); + const preview = await configureGoalAcceptance(store, {...request, dry_run: true}); + const commit = store.commitAuthority.bind(store); + store.commitAuthority = async value => { + store.commitAuthority = commit; + await update(store, "todo_first", {note: "A competing commit after admission"}); + return commit(value); + }; + assert.equal((await configureGoalAcceptance(store, {...request, + review_context_digest: preview.review_context_digest})).status, "conflict"); + assert.equal((await store.readReceipt(String(request.operation_id))).status, "missing"); + assert.equal((await head(store)).head.goal_acceptance, undefined); + }); + test(`${provider}: reviewed context rejects covered work and current-contract changes without effects`, options, async t => { + const store = await fixture(t, provider); await seed(store); + const request = await configureRequest(store); + const preview = await configureGoalAcceptance(store, {...request, dry_run: true}); + await update(store, "todo_first", {text: "New covered requirements"}); + const changed = await head(store); + assert.equal((await configureGoalAcceptance(store, {...request, + review_context_digest: preview.review_context_digest})).reason_code, "goal_acceptance_review_context_mismatch"); + assert.deepEqual(await head(store), changed); + const next = await configureRequest(store); + const nextPreview = await configureGoalAcceptance(store, {...next, dry_run: true}); + assert.equal((await configureGoalAcceptance(store, {...next, operation_id: randomUUID()})).status, "applied"); + const configured = await head(store); + assert.equal((await configureGoalAcceptance(store, {...next, + review_context_digest: nextPreview.review_context_digest})).reason_code, "goal_acceptance_review_context_mismatch"); + assert.deepEqual(await head(store), configured); + }); test(`${provider}: optional oracle source and exact versions survive canonical commit and revoke on changed work`, options, async t => { const store = await fixture(t, provider); await seed(store); const doc = normalizeGoalAcceptanceDocument({scope: {kind: "selected_work", todo_ids: ["todo_first"]}, objective: "Check exact output", non_goals: [],