From 509cc126fac5acf912e05d0a2d0934cf5f61ddd6 Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Sun, 8 Nov 2026 10:06:29 +0800 Subject: [PATCH 1/2] Bind same-batch outcome reviews to their authored result link Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- .../reference/goal-acceptance-observations.md | 16 ++ .../project_lifecycle_refresh_state.py | 5 +- loopx/control_plane/goals/acceptance.py | 5 +- .../goals/acceptance_authority.ts | 17 +- .../goals/outcome_requirements.ts | 21 +- loopx/control_plane/todos/field_update.ts | 12 +- .../control_plane/todos/work_requirements.ts | 10 + .../references/outcome-refinement.md | 8 + .../references/repair-patterns.md | 1 + .../test_outcome_explore_review_basis_cli.py | 195 ++++++++++++++++++ .../goal_outcome_review_work_basis.test.ts | 129 ++++++++++++ 11 files changed, 401 insertions(+), 18 deletions(-) create mode 100644 tests/control_plane/test_outcome_explore_review_basis_cli.py create mode 100644 tests/control_plane_ts/goal_outcome_review_work_basis.test.ts diff --git a/docs/reference/goal-acceptance-observations.md b/docs/reference/goal-acceptance-observations.md index 1c24e573b2..ffb0f2fde0 100644 --- a/docs/reference/goal-acceptance-observations.md +++ b/docs/reference/goal-acceptance-observations.md @@ -132,6 +132,22 @@ runnable research and original budget/stop conditions retain precedence. See the [Agent workflow](../../skills/loopx-project/references/outcome-refinement.md) for inline context, one-call writeback and upgrade/adoption limits. +When one `refresh-state` explicitly supplies both a new complete outcome review +and an Explore result, the review covers that same batch's exact additive Todo +link. The typed owner hashes the prospective link against the original CAS +snapshot; it does not write the link or rebind an old review. The basis becomes +current only after the Todo owner commits the expected link. Failed delivery +remains stale and exact replay retains its original digest. Other work changes +and additional evidence links still invalidate the review. Separately submitted +and legacy completed reviews keep their original basis; no check result or +independent-verification flag changes. + +中文:同一次 `refresh-state` 显式提交完整的新自查与 Explore 结果时,自查依据包含 +该批预期追加的精确 Todo 结果关联。typed owner 基于原 CAS 快照计算摘要,不代替 +Todo owner 写关联,也不替旧自查更新依据。只有预期关联真实提交后依据才是 current; +投递失败保持过期,原样重放保留原摘要。其他工作变化、额外证据关联仍使依据失效。 +分开提交或旧版本已完成的自查保持原义,独立检查与验证标记不变。 + 中文:Goal 概览显示 Agent 登记的支撑成果:原始目的、累计要求、关联工作、来源, 并区分 Agent 自查与独立检查。所有者可原地补充要求并读取原生结果;过期视图先 刷新再重基,未确认写入保留原操作重试。不启动模型或执行检查命令。默认维护和 diff --git a/loopx/cli_commands/project_lifecycle_refresh_state.py b/loopx/cli_commands/project_lifecycle_refresh_state.py index fc9989189f..61589bbf0f 100644 --- a/loopx/cli_commands/project_lifecycle_refresh_state.py +++ b/loopx/cli_commands/project_lifecycle_refresh_state.py @@ -596,7 +596,10 @@ def handle_refresh_state_command( from ..control_plane.goals.acceptance import refine_goal_outcome, public_goal_acceptance refinement_result = refine_goal_outcome( registry_path=registry_path, runtime_root=args.runtime_root, goal_id=args.goal_id, - agent_id=args.agent_id, execute=not args.dry_run, **outcome_update) + agent_id=args.agent_id, execute=not args.dry_run, **outcome_update, + **({"reviewed_work_update": {"todo_id": args.todo_id, + "append_explore_result_node_refs": [explore_result["node_id"]]}} + if explore_result is not None and "review" in outcome_update["refinement"] else {})) outcome_update_result = public_goal_acceptance(refinement_result, include_contract=False) if args.dry_run: # Preview the same ordered composition as execution: the typed diff --git a/loopx/control_plane/goals/acceptance.py b/loopx/control_plane/goals/acceptance.py index 7137059b44..8f639ad4d0 100644 --- a/loopx/control_plane/goals/acceptance.py +++ b/loopx/control_plane/goals/acceptance.py @@ -155,6 +155,7 @@ def refine_goal_outcome( *, registry_path: Path, goal_id: str, refinement: dict[str, Any], expected_provider_revision: str, operation_id: str, agent_id: str | None = None, runtime_root: str | None = None, execute: bool = False, + reviewed_work_update: dict[str, Any] | None = None, ) -> dict[str, Any]: """Record scoped planning intent, never configure validators or attest success.""" registry_path, goal, route = _routing(registry_path, goal_id, runtime_root, agent_id) @@ -165,7 +166,9 @@ def refine_goal_outcome( raise ValueError("Outcome refinement requires an active Goal") return _result("goal.acceptance.refine", {**route, "actor_agent_id": agent_id, "expected_provider_revision": expected_provider_revision, "operation_id": operation_id, - "refinement": refinement, "dry_run": not execute}, timeout=CANONICAL_AUTHORITY_WRITE_TIMEOUT_SECONDS) + "refinement": refinement, "dry_run": not execute, + **({"reviewed_work_update": reviewed_work_update} if reviewed_work_update is not None else {}), + }, timeout=CANONICAL_AUTHORITY_WRITE_TIMEOUT_SECONDS) def update_goal_objective( diff --git a/loopx/control_plane/goals/acceptance_authority.ts b/loopx/control_plane/goals/acceptance_authority.ts index 7fb57339b4..13c02d281f 100644 --- a/loopx/control_plane/goals/acceptance_authority.ts +++ b/loopx/control_plane/goals/acceptance_authority.ts @@ -197,19 +197,28 @@ async function commit(store: AuthorityStore, request: JsonObject, head: Authorit /** Append planning requirements without configuring checks or changing Goal/Todo intent. */ export async function refineGoalAcceptanceOutcome(store: AuthorityStore, value: JsonObject): Promise { const request = canonicalAuthorityObject(value, "outcome refinement request"); - acceptanceKeys(request, [...REQUEST_FIELDS, "refinement"], [...LOCAL_FIELDS, "goal_ref"]); + acceptanceKeys(request, [...REQUEST_FIELDS, "refinement"], [...LOCAL_FIELDS, "goal_ref", "reviewed_work_update"]); for (const key of ["goal_id", "operation_id", "expected_provider_revision"]) requireAuthorityStoreId(request[key], key); acceptanceRequire(request.actor_agent_id === null || typeof request.actor_agent_id === "string" && !!request.actor_agent_id, "outcome actor is required"); acceptanceRequire(request.dry_run === undefined || typeof request.dry_run === "boolean", "dry_run must be boolean"); const command = receiptFor(request, "refine"); + const readback = async (result: JsonObject): Promise => { + // Older refreshes stored the same review before considering their later + // attachment. Exact legacy recovery is read-only; never upgrade its basis. + if (result.reason_code !== "coordination_operation_identity_mismatch" || request.reviewed_work_update === undefined) return result; + const {reviewed_work_update: _update, ...legacyRequest} = request; + const legacy = await receiptFor(legacyRequest, "refine").receipt.read(store); + return legacy?.status === "replayed" ? legacy : result; + }; const replay = await command.receipt.read(store); - if (replay) return replayWithCurrentProjection(store, replay, String(request.goal_id)); + if (replay) return replayWithCurrentProjection(store, await readback(replay), String(request.goal_id)); const observation = await command.receipt.observe(store); - if (observation.kind === "receipt") return replayWithCurrentProjection(store, observation.result, String(request.goal_id)); + if (observation.kind === "receipt") return replayWithCurrentProjection(store, await readback(observation.result), String(request.goal_id)); const head = current(observation.authority, request); if (!loaded(head)) return source(store, head); const goalId = String(request.goal_id), goalRef = requestGoalRef(request); - const state = refineGoalOutcome(head.head, goalId, goalRef, request.actor_agent_id as string | null, request.refinement); + const state = refineGoalOutcome(head.head, goalId, goalRef, request.actor_agent_id as string | null, request.refinement, + request.reviewed_work_update); const next: JsonObject = {...head.head, goal_outcomes: state}; // Bind a first exact-lifetime declaration to the same lifecycle owner as checks. if (goalRef !== null && readGoalAcceptanceLifecycle(head.head, goalId) === null) { diff --git a/loopx/control_plane/goals/outcome_requirements.ts b/loopx/control_plane/goals/outcome_requirements.ts index 32f73cfedb..411aed3411 100644 --- a/loopx/control_plane/goals/outcome_requirements.ts +++ b/loopx/control_plane/goals/outcome_requirements.ts @@ -9,6 +9,7 @@ import {BARE_SHA256_PATTERN} from "../content_digest.ts"; import {goalAcceptanceTodoDigest} from "./acceptance_work_basis.ts"; import {projectRelations, todoRef} from "../work_items/planning_relations.ts"; import {normalizeTodoResumeWhen, TODO_RESUME_NORMALIZE_REQUEST_SCHEMA_VERSION} from "../todos/resume_condition.ts"; +import {appendTodoExploreReferences} from "../todos/work_requirements.ts"; export const GOAL_OUTCOMES_SCHEMA = "loopx_goal_outcomes_v0"; export const OUTCOME_CONTEXT_NAVIGATION_SCHEMA = "outcome_context_navigation_v1"; @@ -126,7 +127,7 @@ function outcomeBytes(state: OutcomeState): {declaration: number; review: number } export function refineGoalOutcome(head: JsonObject, goalId: string, goalRef: WireExactGoalRef | null, - actor: string | null, value: unknown): OutcomeState { + actor: string | null, value: unknown, reviewedWorkUpdate?: unknown): OutcomeState { let previous = readState(head, goalId); const lifecycle = readGoalAcceptanceLifecycle(head, goalId); requireValue(!lifecycle || lifecycle.state === "active" && goalRef && sameExactGoalRef(lifecycle.goal_ref, goalRef), "outcome Goal lifetime is unavailable or changed"); @@ -144,6 +145,8 @@ export function refineGoalOutcome(head: JsonObject, goalId: string, goalRef: Wir } const patch = canonicalAuthorityObject(value, "outcome refinement"); keys(patch, ["outcome_id"], ["source", "owner_agent_id", "objective", "requirements", "todo_ids", "review"]); + requireValue(reviewedWorkUpdate === undefined || patch.review !== undefined, + "a reviewed work update requires a newly authored complete review"); const outcomeId = id(patch.outcome_id, "outcome id"); const old = previous?.outcomes.find(o => o.id === outcomeId); const owner = id(patch.owner_agent_id === undefined ? old?.owner_agent_id : patch.owner_agent_id, "outcome Agent"); @@ -181,7 +184,21 @@ export function refineGoalOutcome(head: JsonObject, goalId: string, goalRef: Wir if (patch.review !== undefined) { const items = reviewItems(patch.review); requireValue(items.length === outcome.requirements.length && items.every(row => reqs.has(row.id)), "review must address every retained requirement exactly once"); - outcome.review = {basis_digest: reviewBasis(outcome, todos), items}; + let reviewedTodos = todos; + if (reviewedWorkUpdate !== undefined) { + // The host transports only the explicit same-batch additive effect. Hash + // its exact prospective work state, never bless a later arbitrary head. + // Until the Todo owner commits that effect this review remains stale. + const update = canonicalAuthorityObject(reviewedWorkUpdate, "reviewed work update"); + keys(update, ["todo_id", "append_explore_result_node_refs"]); + const todoId = id(update.todo_id, "reviewed Todo id"), todo = todos.get(todoId); + requireValue(actor !== null && todo && todo.role === "agent" && todo.claimed_by === actor + && outcome.todo_ids.includes(todoId), "reviewed work must be the caller's linked Todo"); + requireValue(Array.isArray(update.append_explore_result_node_refs) && update.append_explore_result_node_refs.length === 1, + "reviewed work must name exactly one additive result link"); + reviewedTodos = new Map(todos).set(todoId, {...todo, ...appendTodoExploreReferences(todo, update.append_explore_result_node_refs)}); + } + outcome.review = {basis_digest: reviewBasis(outcome, reviewedTodos), items}; } // Refinement changes one outcome, not the positions of unrelated requirements // in canonical snapshots and their derived full/delta context. diff --git a/loopx/control_plane/todos/field_update.ts b/loopx/control_plane/todos/field_update.ts index d4f4c3edc6..64362b10a1 100644 --- a/loopx/control_plane/todos/field_update.ts +++ b/loopx/control_plane/todos/field_update.ts @@ -1,6 +1,6 @@ /** Pure field intent planning. Admission, leases, validation and commit stay * with the calling lifecycle transaction; this result grants no write right. */ -import { normalizeTodoWorkRequirements } from "./work_requirements.ts"; +import { appendTodoExploreReferences, normalizeTodoWorkRequirements } from "./work_requirements.ts"; import {planTodoPriority} from "./priority.ts"; import type { JsonObject } from "../effect_program.ts"; import { EffectRuntimeRequestError } from "../effect_runtime_errors.ts"; @@ -215,15 +215,7 @@ export function planTodoFieldUpdate(value: unknown): TodoFieldUpdatePlan { if (present(intent.explore_result_node_refs)) { throw new EffectRuntimeRequestError("cannot replace and append Explore references together"); } - const additions = normalizeTodoWorkRequirements({ - explore_result_node_refs: intent.append_explore_result_node_refs, - }).explore_result_node_refs as string[]; - const existing = normalizeTodoWorkRequirements({ - explore_result_node_refs: block.explore_result_node_refs ?? [], - }).explore_result_node_refs as string[]; - Object.assign(updates, normalizeTodoWorkRequirements({ - explore_result_node_refs: [...existing, ...additions], - })); + Object.assign(updates, appendTodoExploreReferences(block, intent.append_explore_result_node_refs)); } // Public update carries the effective scope and raw observation once. The // field plan composes validation and generation without another RPC. diff --git a/loopx/control_plane/todos/work_requirements.ts b/loopx/control_plane/todos/work_requirements.ts index 5070002a09..39f57fa261 100644 --- a/loopx/control_plane/todos/work_requirements.ts +++ b/loopx/control_plane/todos/work_requirements.ts @@ -127,3 +127,13 @@ export function normalizeTodoWorkRequirements(intent: JsonObject): JsonObject { } return result; } + +/** One additive effect, shared by the committing Todo owner and prospective + * review bases. Calculating the fields never grants authority or writes them. */ +export function appendTodoExploreReferences(todo: JsonObject, additions: unknown): JsonObject { + const refs = normalizeTodoWorkRequirements({explore_result_node_refs: requireStringArray(additions, "explore_result_node_refs")}) + .explore_result_node_refs as string[]; + const existing = normalizeTodoWorkRequirements({explore_result_node_refs: todo.explore_result_node_refs ?? []}) + .explore_result_node_refs as string[]; + return normalizeTodoWorkRequirements({explore_result_node_refs: [...existing, ...refs]}); +} diff --git a/skills/loopx-project/references/outcome-refinement.md b/skills/loopx-project/references/outcome-refinement.md index c495b097bf..9af89712e1 100644 --- a/skills/loopx-project/references/outcome-refinement.md +++ b/skills/loopx-project/references/outcome-refinement.md @@ -141,6 +141,14 @@ For changed requirements or reviews, add this envelope to the **existing** its ordinary routing, Turn, checkpoint and delivery arguments. This does not replace them. +If that same writeback includes an explicit Explore result, a newly authored +complete review binds the exact additive result link expected from this batch. +It remains stale until the Todo owner commits the link; failed delivery replays +the original digest. Other work/evidence changes still require review. Do not +repeat an unchanged review solely to acknowledge a successfully delivered +same-batch link. Separately submitted and legacy completed reviews are not +automatically rebound, and independent checks retain their authority. + An enabled due Stride review binds the delivered context, which includes outcome facts. When this milestone changes those facts, apply the refinement first with the standalone command below, then re-enter the guard with the same Turn identity diff --git a/skills/loopx-self-repair/references/repair-patterns.md b/skills/loopx-self-repair/references/repair-patterns.md index 3b1333365b..900bf74c94 100644 --- a/skills/loopx-self-repair/references/repair-patterns.md +++ b/skills/loopx-self-repair/references/repair-patterns.md @@ -5,6 +5,7 @@ teaches a reusable control-plane lesson. | Pattern | Symptoms | Evidence To Read | Likely Root | Durable Repair | | --- | --- | --- | --- | --- | +| `outcome_review_same_batch_link_invalidation` | A successful review-plus-result refresh immediately reports its Agent review basis stale; an unchanged standalone review is then repeated. | Exact refinement and result-delivery receipts, original CAS revision, retained conclusions and canonical Todo links before/after; distinguish the authored effect from other new evidence. | Refinement hashes pre-link work, then post-writeback delivery adds a work-basis input. | Bind only the newly authored complete review to the exact normalized same-batch additive link using the existing typed append rule and original CAS snapshot. Keep it stale until actual commit. Preserve independent verification, all retained requirements, separate/legacy reviews, initial CAS refusal, failed delivery, exact receipt replay and concurrent work/evidence invalidation through real File/SQLite CLI. Never exclude all evidence links, refresh old review digests against an arbitrary later head, reorder effects before primary admission or claim model/score benefit from this repair. | | `answer_question_replaced_by_delivery_status` | A user asks why a result occurred or whether it is expected, but receives measurements, tests or PR progress even though the reply was delivered completely. | The actual question and relevant corrections, current receiver brief, shared answer guidance, and the complete message on its original return route. | The author answered implementation progress instead of the causal question; successful transport or correct counters were mistaken for a useful answer. | Retain the correction in the existing Task and outcome, then repair the existing shared generation guidance: explain meaning, supported mechanism, expectations and the specific unknown before supporting data. Keep hypotheses separate from confirmed causes and preserve the qualified investigation owner. Verify guidance at native callers and read back the actual explanatory answer. Do not add keyword-based answer scoring, a second model rewrite, another display or a duplicate investigation; a prompt test or delivery receipt does not prove model compliance or user adoption. | | `queue_action_type_coercion` | A queued input action such as `["edit"]` is accepted but applies cancellation, or a stored operation accepts a non-string action. | Original JSON types, the typed planner and receipt decoder, real HTTP responses, canonical Turn/transcript/events/Session snapshots and native IO before/after refusal. | Membership checks stringify the action while effect selection and replay compare its original literal value. | Validate the original literal action at the existing owner before branching and when decoding pending or committed receipts. Cover arrays, nested arrays, objects, nulls, numbers, booleans and unknown strings; require rejection without state or native effects, then verify legal edit/cancel, original-operation replay and queue races. Preserve malformed historical evidence rather than coercing or rewriting receipts. | | `collaboration_return_format_projection_gap` | A complete answer that fits the result budget is rejected, or a published versioned return becomes unreadable or cannot replay through the real CLI. | Selected renderer output and Unicode count; persisted reply, compact publication/Inbox observations, original-route delivery and legacy replay at the same boundary. | Validation counted a different renderer before choosing the immutable format, or a compact codec omitted the persisted format marker and silently treated it as legacy. | Apply the unchanged budget to the actual selected format, carry its existing marker through every compact observation, and retain old bytes on replay. Validate BMP/non-BMP exact-limit and one-character-over cases, unsupported versions, real CLI publication/retry, original-audience return and File/SQLite packaged readback. Do not raise the budget, truncate the answer, rewrite historical receipts or add a second model. | diff --git a/tests/control_plane/test_outcome_explore_review_basis_cli.py b/tests/control_plane/test_outcome_explore_review_basis_cli.py new file mode 100644 index 0000000000..dc6347ca54 --- /dev/null +++ b/tests/control_plane/test_outcome_explore_review_basis_cli.py @@ -0,0 +1,195 @@ +"""Same-batch reviews cover the authored link, never unrelated later evidence.""" +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from canonical_authority_fixture import isolate_sqlite_runtime +from tests.control_plane.test_explore_result_writeback import ATTACHMENT +from tests.control_plane.test_quota_settlement_cli import ( + AGENT_ID, GOAL_ID, TODO_ID, TURN_ID, _initialize_fixture_authority, + _run_cli, _spend_run_count, _write_fixture, +) + + +def _batch_fixture(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, provider: str): + isolate_sqlite_runtime(tmp_path, monkeypatch) + project, runtime, registry = _write_fixture(tmp_path) + config = json.loads(registry.read_text()) + config["goals"][0].update(status="active", explore_graph={"enabled": True}) + registry.write_text(json.dumps(config)) + _initialize_fixture_authority(project, runtime, provider=provider) + + def call(*args: str, expected_code: int = 0) -> dict: + code, result = _run_cli(registry, runtime, *args, cwd=project) + assert code == expected_code, result + return result + + def inspect() -> dict: + return call("goal-acceptance", "inspect", "--goal-id", GOAL_ID) + + requirements = [{"id": key, "description": description} for key, description in ( + ("content", "Preserve the requested content"), + ("recovery", "Validate interrupted recovery"), + ("scope", "Keep conclusions within the tested scope"), + )] + declaration = {"outcome_id": "export", "owner_agent_id": AGENT_ID, + "objective": "Deliver a recoverable export", "todo_ids": [TODO_ID], + "requirements": requirements, + "source": {"ref": "message:export", "summary": "The owner requires a recoverable export"}} + document = tmp_path / "declaration.json" + document.write_text(json.dumps(declaration)) + call("goal-acceptance", "refine", "--goal-id", GOAL_ID, "--agent-id", AGENT_ID, + "--document", str(document), "--operation-id", "declare-export", + "--expected-provider-revision", inspect()["provider_revision"], "--execute") + binding = ("--goal-id", GOAL_ID, "--agent-id", AGENT_ID, "--todo-id", TODO_ID, + "--turn-instance-id", TURN_ID) + guard = call("quota", "should-run", "--codex-app", *binding) + assert guard["should_run"] + reviews = [{"id": row["id"], "conclusion": verdict, + "evidence_refs": ATTACHMENT["evidence_refs"], "note": "Read the result within its tested scope"} + for row, verdict in zip(requirements, ("met", "unmet", "uncertain"), strict=True)] + update = tmp_path / "review.json" + update.write_text(json.dumps({"operation_id": "export-review", + "expected_provider_revision": inspect()["provider_revision"], + "refinement": {"outcome_id": "export", "review": reviews}})) + attachment = tmp_path / "result.json" + attachment.write_text(json.dumps(ATTACHMENT)) + batch = ("refresh-state", *binding, "--classification", "validated_change", + "--delivery-batch-scale", "implementation", "--delivery-outcome", "outcome_progress", + "--vision-unchanged-reason", "The result preserves the current validation plan", + "--no-global-sync", "--suppress-external-sinks", "--goal-outcome-update-json", str(update), + "--explore-result-json", str(attachment)) + return project, runtime, registry, call, inspect, declaration, reviews, batch + + +@pytest.mark.parametrize("provider", ["file", "sqlite"]) +@pytest.mark.parametrize("source_mode", ["file", "path_delta"]) +def test_batch_review_is_bound_to_exact_authored_link_without_new_review( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, provider: str, source_mode: str, +) -> None: + _, runtime, _, call, inspect, declaration, reviews, batch = _batch_fixture(tmp_path, monkeypatch, provider) + if source_mode == "path_delta": + vision = tmp_path / "vision.json" + vision.write_text(json.dumps({"schema_version": "goal_vision_replan_contract_v0", + "state": "vision_patch_proposed", "vision_patch": {"acceptance_summary": "Validate the export within its tested scope"}, + "path_delta": {"outcome": "continue", "prior_assumption": "The result needed scope validation", + "observed_reality": ATTACHMENT["observation"], "changed": [ATTACHMENT["interpretation"]], + "evidence_refs": ATTACHMENT["evidence_refs"]}, + "explore_result": {"schema_version": "explore_result_from_path_delta_v0", + **{key: ATTACHMENT[key] for key in ("node_id", "question", "applicability", "input_revision", "status")}}})) + for option in ("--vision-unchanged-reason", "--explore-result-json"): + position = batch.index(option) + batch = batch[:position] + batch[position + 2:] + batch += ("--agent-vision-json", str(vision)) + batch += ("--settle-turn", "--settlement-receipt") + before = inspect() + preview = call(*batch, "--dry-run") + assert preview["goal_outcome_update"]["status"] == "planned" + assert inspect() == before + result = call(*batch) + assert result["explore_result_delivery"]["ok"] + assert _spend_run_count(runtime) == 1 + outcome = inspect()["goal_acceptance_contract"]["outcome_requirements"]["outcomes"][0] + assert outcome["revision"] == 2, "one authored review must not require another refinement" + assert outcome["sources"] == [declaration["source"]] + assert [{"id": r["id"], "description": r["description"]} for r in outcome["requirements"]] == declaration["requirements"] + for row, review in zip(outcome["requirements"], reviews, strict=True): + assert row["agent_review"] == {**review, "basis_current": True, "independently_verified": False} + assert row["status"] == "unverified" + index = runtime / "goals" / GOAL_ID / "runs" / "index.jsonl" + committed = inspect(), index.read_bytes() + replay = call(*batch) + assert replay["goal_outcome_update"]["status"] == "replayed" + assert replay["explore_result_delivery"]["idempotent_replay"] + assert (inspect(), index.read_bytes()) == committed + assert _spend_run_count(runtime) == 1 + + +@pytest.mark.parametrize("provider", ["file", "sqlite"]) +@pytest.mark.parametrize("concurrent", [False, True]) +def test_interrupted_batch_stays_stale_until_exact_link_and_preserves_concurrent_evidence( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str], + provider: str, concurrent: bool, +) -> None: + from loopx.cli import main + import loopx.capabilities.explore.result_writeback as writeback + + project, runtime, registry, call, inspect, _, reviews, batch = _batch_fixture(tmp_path, monkeypatch, provider) + original = writeback.update_goal_todo + + def interrupt_link(**kwargs): + if kwargs.get("dry_run"): + return original(**kwargs) + if concurrent: + original(**{**kwargs, "append_explore_result_node_refs": ["unreviewed-result"]}) + raise OSError("interrupted after graph persistence") + + with monkeypatch.context() as patch: + patch.chdir(project) + patch.setattr(writeback, "update_goal_todo", interrupt_link) + code = main(["--registry", str(registry), "--runtime-root", str(runtime), "--format", "json", *batch]) + result = json.loads(capsys.readouterr().out) + assert code == 1 and result["appended"] and not result["explore_result_delivery"]["ok"], json.dumps(result) + interrupted = inspect()["goal_acceptance_contract"]["outcome_requirements"]["outcomes"][0] + assert interrupted["revision"] == 2 + assert all(not row["agent_review"]["basis_current"] for row in interrupted["requirements"]) + # Replay in a new CLI process: it recovers the original digest, never hashes + # whatever authority head happens to be current after a failed delivery. + recovered = call(*batch) + assert recovered["goal_outcome_update"]["status"] == "replayed" + assert recovered["explore_result_delivery"]["ok"] + after = inspect()["goal_acceptance_contract"]["outcome_requirements"]["outcomes"][0] + assert after["revision"] == 2 + for row, review in zip(after["requirements"], reviews, strict=True): + assert row["agent_review"] == {**review, "basis_current": not concurrent, "independently_verified": False} + todo = call("todo", "list", "--goal-id", GOAL_ID, "--todo-id", TODO_ID)["todo"] + assert todo["explore_result_node_refs"] == (["unreviewed-result"] if concurrent else []) + [ATTACHMENT["node_id"]] + retained = inspect() + call(*batch) + assert inspect() == retained + + +@pytest.mark.parametrize("provider", ["file", "sqlite"]) +def test_stale_initial_cas_refuses_review_and_attachment( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, provider: str, +) -> None: + from loopx.todos import update_goal_todo + + _, runtime, registry, call, inspect, _, _, batch = _batch_fixture(tmp_path, monkeypatch, provider) + update_goal_todo(registry_path=registry, runtime_root_arg=str(runtime), goal_id=GOAL_ID, + todo_id=TODO_ID, agent_id=AGENT_ID, append_explore_result_node_refs=["unreviewed-result"]) + before = inspect() + refused = call(*batch, expected_code=1) + assert not refused["appended"] and "goal_outcome_update" not in refused + assert inspect() == before + from loopx.capabilities.explore.result_log import explore_result_log_path + assert not explore_result_log_path(runtime, GOAL_ID).exists() + + +@pytest.mark.parametrize("provider", ["file", "sqlite"]) +@pytest.mark.parametrize("legacy_batch", [False, True]) +def test_separate_or_legacy_review_does_not_adopt_new_attachment( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, provider: str, legacy_batch: bool, +) -> None: + from loopx.control_plane.goals.acceptance import refine_goal_outcome + + _, runtime, registry, call, inspect, _, _, batch = _batch_fixture(tmp_path, monkeypatch, provider) + update_index = batch.index("--goal-outcome-update-json") + update = json.loads(Path(batch[update_index + 1]).read_text()) + # Represents a separately reviewed state, including a committed operation + # from an older runtime whose batched request did not anticipate the link. + refine_goal_outcome(registry_path=registry, runtime_root=str(runtime), goal_id=GOAL_ID, + agent_id=AGENT_ID, execute=True, **update) + before = inspect()["goal_acceptance_contract"]["outcome_requirements"]["outcomes"][0] + assert all(row["agent_review"]["basis_current"] for row in before["requirements"]) + args = batch if legacy_batch else batch[:update_index] + batch[update_index + 2:] + result = call(*args) + assert result["explore_result_delivery"]["ok"] + if legacy_batch: + assert result["goal_outcome_update"]["status"] == "replayed" + after = inspect()["goal_acceptance_contract"]["outcome_requirements"]["outcomes"][0] + assert after["revision"] == before["revision"] == 2 + assert all(not row["agent_review"]["basis_current"] for row in after["requirements"]) diff --git a/tests/control_plane_ts/goal_outcome_review_work_basis.test.ts b/tests/control_plane_ts/goal_outcome_review_work_basis.test.ts new file mode 100644 index 0000000000..e2259cb027 --- /dev/null +++ b/tests/control_plane_ts/goal_outcome_review_work_basis.test.ts @@ -0,0 +1,129 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import {mkdtemp, rm} from "node:fs/promises"; +import {tmpdir} from "node:os"; +import {join} from "node:path"; +import type {JsonObject} from "../../loopx/control_plane/effect_program.ts"; +import {FileAuthorityStore} from "../../loopx/control_plane/coordination/file_authority_store.ts"; +import {SqliteAuthorityStore} from "../../loopx/control_plane/coordination/sqlite_authority_store.ts"; +import {authorityProjectionFixture} from "./authority_projection_fixture.ts"; +import {refineGoalOutcome, projectGoalOutcomes} from "../../loopx/control_plane/goals/outcome_requirements.ts"; +import {refineGoalAcceptanceOutcome, inspectGoalAcceptance} from "../../loopx/control_plane/goals/acceptance_authority.ts"; +import {appendTodoExploreReferences} from "../../loopx/control_plane/todos/work_requirements.ts"; +import {planTodoFieldUpdate, TODO_FIELD_UPDATE_REQUEST_SCHEMA} from "../../loopx/control_plane/todos/field_update.ts"; + +const goal = "review-goal"; +const todoId = "todo_export"; +const node = "question_result"; +const update = {todo_id: todoId, append_explore_result_node_refs: [node]}; +const declaration = {outcome_id: "export", owner_agent_id: "author", objective: "Deliver a recoverable export", + requirements: [{id: "content", description: "Preserve the requested content"}], todo_ids: [todoId, "todo_recovery"], + source: {ref: "message:export", summary: "The owner requested a recoverable export"}}; +const review = {outcome_id: "export", review: [{id: "content", conclusion: "met", evidence_refs: ["artifact:export"], + note: "Read the content within its tested scope"}]}; + +function fixture(): JsonObject { + const head = authorityProjectionFixture(goal, [todoId, "todo_recovery", "todo_unrelated"].map(todo_id => ({todo_id, + role: "agent", status: "open", done: false, archive_state: "active", claimed_by: "author", + task_class: "advancement_task", text: "Validate the declared export"}))); + head.goal_outcomes = refineGoalOutcome(head, goal, null, "author", declaration); + return head; +} +function withTodos(head: JsonObject, change: (row: JsonObject) => JsonObject): JsonObject { + return authorityProjectionFixture(goal, (head.todos as JsonObject[]).map(change), [], "native", head); +} +function current(head: JsonObject): boolean { + const outcome = (projectGoalOutcomes(head, goal, {})!.outcomes as JsonObject[])[0]; + const requirement = (outcome.requirements as JsonObject[])[0]; + assert.equal((requirement.agent_review as JsonObject).independently_verified, false); + assert.equal(requirement.status, "unverified"); + return (requirement.agent_review as JsonObject).basis_current === true; +} +function linked(head: JsonObject): JsonObject { + return withTodos(head, row => row.todo_id === todoId ? {...row, ...appendTodoExploreReferences(row, [node])} : row); +} + +test("shared append preserves normalization, refusal details and validation precedence", () => { + const plan = (refs: unknown, additions: unknown) => planTodoFieldUpdate({schema_version: TODO_FIELD_UPDATE_REQUEST_SCHEMA, + todo: {todo_id: todoId, status: "open", explore_result_node_refs: refs}, + intent: {append_explore_result_node_refs: additions}, updated_at: "2026-01-01T00:00:00Z"}); + assert.deepEqual((plan(["prior"], [" next ", "next"]).metadata_updates as JsonObject).explore_result_node_refs, ["prior", "next"]); + assert.deepEqual((plan(["prior"], []).metadata_updates as JsonObject).explore_result_node_refs, ["prior"]); + for (const refs of [[], ["/invalid"]]) assert.throws(() => plan(refs, 5), { + message: "explore_result_node_refs must be an array of strings"}); + assert.throws(() => plan(["/invalid"], ["valid"]), {message: "explore_result_node_refs requires valid Explore node ids"}); +}); + +test("prospective review covers only the exact additive effect and never mutates the source", () => { + const original = fixture(), retained = structuredClone(original); + const reviewed = {...original, goal_outcomes: refineGoalOutcome(original, goal, null, "author", review, update)}; + assert.deepEqual(original, retained); + assert.equal(current(reviewed), false, "a planned link is not a committed effect"); + assert.equal(current(linked(reviewed)), true); + for (const [target, fields] of [ + [todoId, {explore_result_node_refs: [node, "question_unreviewed"]}], + [todoId, {text: "Validate a different export"}], + [todoId, {required_capabilities: ["shell"]}], + ["todo_recovery", {text: "Changed linked recovery work"}], + ] as [string, JsonObject][]) { + const changed = withTodos(linked(reviewed), row => row.todo_id === target ? {...row, ...fields} : row); + assert.equal(current(changed), false, "a concurrent semantic change cannot enter the reviewed basis"); + } + assert.equal(current(withTodos(linked(reviewed), row => row.todo_id === "todo_unrelated" + ? {...row, text: "Unrelated work can continue"} : row)), true); + const ordinary = {...original, goal_outcomes: refineGoalOutcome(original, goal, null, "author", review)}; + assert.equal(current(ordinary), true); + assert.equal(current(linked(ordinary)), false, "separately reviewed state must not adopt a later link"); +}); + +test("reviewed updates cannot rebind old reviews, replace work or act on unlinked/foreign work", () => { + const head = fixture(); + for (const invalid of [ + {...update, status: "done"}, {...update, todo_id: "todo_unrelated"}, + {...update, todo_id: "todo_missing"}, {...update, append_explore_result_node_refs: []}, + {...update, append_explore_result_node_refs: [node, "question_other"]}, + {...update, append_explore_result_node_refs: ["/local/path"]}, + ]) assert.throws(() => refineGoalOutcome(head, goal, null, "author", review, invalid)); + assert.throws(() => refineGoalOutcome(head, goal, null, null, review, update)); + assert.throws(() => refineGoalOutcome(head, goal, null, "other", review, update)); + assert.throws(() => refineGoalOutcome(head, goal, null, "author", {outcome_id: "export"}, update)); + const foreign = withTodos(head, row => row.todo_id === todoId ? {...row, claimed_by: "other"} : row); + assert.throws(() => refineGoalOutcome(foreign, goal, null, "author", review, update)); +}); + +for (const provider of ["file", "sqlite"] as const) test(`${provider}: exact CAS and receipts retain anticipated basis across restart and lost replies`, async t => { + const root = await mkdtemp(join(tmpdir(), "outcome-reviewed-work-")); + t.after(() => rm(root, {recursive: true, force: true})); + const open = () => provider === "file" ? new FileAuthorityStore(root, goal) : new SqliteAuthorityStore(root, goal); + const store = open(); + await store.commitAuthority({operation_id: "seed", expected_provider_revision: null, next_projection: fixture(), events: [], receipts: []}); + const before = await store.loadAuthority(); assert.equal(before.status, "loaded"); if (before.status !== "loaded") return; + const request = {goal_id: goal, actor_agent_id: "author", operation_id: "review", expected_provider_revision: before.provider_revision, + refinement: review, reviewed_work_update: update}; + assert.equal((await refineGoalAcceptanceOutcome(store, {...request, dry_run: true})).status, "planned"); + assert.deepEqual(await store.loadAuthority(), before); + const commit = store.commitAuthority.bind(store); + store.commitAuthority = async value => {await commit(value); throw new Error("reply lost after commit");}; + assert.equal((await refineGoalAcceptanceOutcome(store, request)).status, "recovered"); + const saved = await open().loadAuthority(); assert.equal(saved.status, "loaded"); if (saved.status !== "loaded") return; + assert.equal(current(saved.head), false); + assert.equal((await refineGoalAcceptanceOutcome(open(), request)).status, "replayed"); + assert.deepEqual(await open().loadAuthority(), saved); + const changed = await refineGoalAcceptanceOutcome(open(), {...request, + reviewed_work_update: {...update, append_explore_result_node_refs: ["question_different"]}}); + assert.equal(changed.reason_code, "coordination_operation_identity_mismatch"); + assert.deepEqual(await open().loadAuthority(), saved); + // An unrelated authority commit never authorizes silently rebasing a fresh + // operation. The original CAS is preserved even when its prospective link fits. + const refused = await refineGoalAcceptanceOutcome(open(), {...request, operation_id: "stale-review"}); + assert.equal(refused.status, "conflict"); + assert.deepEqual(await open().loadAuthority(), saved); + await open().commitAuthority({operation_id: "todo-link", expected_provider_revision: saved.provider_revision, + next_projection: linked(saved.head), events: [], receipts: []}); + const final = await open().loadAuthority(); assert.equal(final.status, "loaded"); if (final.status !== "loaded") return; + assert.equal(current(final.head), true); + assert.equal((await refineGoalAcceptanceOutcome(open(), request)).status, "replayed"); + assert.deepEqual(await open().loadAuthority(), final); + const contract = (await inspectGoalAcceptance(open(), goal)).goal_acceptance_contract as JsonObject; + assert.equal((contract.outcome_requirements as JsonObject).revision, 2); +}); From 3b7534ab038be611c7cdd4de671a5d4a0642b37d Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Sun, 8 Nov 2026 10:23:49 +0800 Subject: [PATCH 2/2] Preserve separate outcome reviews during result delivery Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- .../reference/goal-acceptance-observations.md | 13 +++++--- .../goals/outcome_requirements.ts | 9 +++-- .../references/outcome-refinement.md | 7 ++-- .../references/repair-patterns.md | 2 +- .../test_outcome_explore_review_basis_cli.py | 33 +++++++++++++++++-- .../goal_outcome_review_work_basis.test.ts | 25 ++++++++++++-- 6 files changed, 75 insertions(+), 14 deletions(-) diff --git a/docs/reference/goal-acceptance-observations.md b/docs/reference/goal-acceptance-observations.md index ffb0f2fde0..32c5459537 100644 --- a/docs/reference/goal-acceptance-observations.md +++ b/docs/reference/goal-acceptance-observations.md @@ -133,8 +133,11 @@ See the [Agent workflow](../../skills/loopx-project/references/outcome-refinemen for inline context, one-call writeback and upgrade/adoption limits. When one `refresh-state` explicitly supplies both a new complete outcome review -and an Explore result, the review covers that same batch's exact additive Todo -link. The typed owner hashes the prospective link against the original CAS +and an Explore result for work linked to that outcome, the review covers that +same batch's exact additive Todo link. A result for separate same-Agent work +does not change the reviewed outcome's basis or work links. The typed owner +validates the complete result-link intent and ownership in either case, then +hashes a relevant prospective link against the original CAS snapshot; it does not write the link or rebind an old review. The basis becomes current only after the Todo owner commits the expected link. Failed delivery remains stale and exact replay retains its original digest. Other work changes @@ -142,8 +145,10 @@ and additional evidence links still invalidate the review. Separately submitted and legacy completed reviews keep their original basis; no check result or independent-verification flag changes. -中文:同一次 `refresh-state` 显式提交完整的新自查与 Explore 结果时,自查依据包含 -该批预期追加的精确 Todo 结果关联。typed owner 基于原 CAS 快照计算摘要,不代替 +中文:同一次 `refresh-state` 显式提交完整的新自查,以及该 outcome 关联工作的 +Explore 结果时,自查依据包含该批预期追加的精确 Todo 结果关联。同 Agent 的其他 +工作结果不改变本 outcome 的依据或关联;typed owner 在两种情况下均先验证完整 +意图和归属,再基于原 CAS 快照计算相关工作的摘要,不代替 Todo owner 写关联,也不替旧自查更新依据。只有预期关联真实提交后依据才是 current; 投递失败保持过期,原样重放保留原摘要。其他工作变化、额外证据关联仍使依据失效。 分开提交或旧版本已完成的自查保持原义,独立检查与验证标记不变。 diff --git a/loopx/control_plane/goals/outcome_requirements.ts b/loopx/control_plane/goals/outcome_requirements.ts index 411aed3411..b567763c0e 100644 --- a/loopx/control_plane/goals/outcome_requirements.ts +++ b/loopx/control_plane/goals/outcome_requirements.ts @@ -192,11 +192,14 @@ export function refineGoalOutcome(head: JsonObject, goalId: string, goalRef: Wir const update = canonicalAuthorityObject(reviewedWorkUpdate, "reviewed work update"); keys(update, ["todo_id", "append_explore_result_node_refs"]); const todoId = id(update.todo_id, "reviewed Todo id"), todo = todos.get(todoId); - requireValue(actor !== null && todo && todo.role === "agent" && todo.claimed_by === actor - && outcome.todo_ids.includes(todoId), "reviewed work must be the caller's linked Todo"); + requireValue(actor !== null && todo && todo.role === "agent" && todo.claimed_by === actor, + "reviewed work must be the caller's Todo"); requireValue(Array.isArray(update.append_explore_result_node_refs) && update.append_explore_result_node_refs.length === 1, "reviewed work must name exactly one additive result link"); - reviewedTodos = new Map(todos).set(todoId, {...todo, ...appendTodoExploreReferences(todo, update.append_explore_result_node_refs)}); + const prospectiveTodo = {...todo, ...appendTodoExploreReferences(todo, update.append_explore_result_node_refs)}; + // A batch may review another outcome while delivering this Todo's result. + // Validate the complete intent first; only linked work enters this basis. + if (outcome.todo_ids.includes(todoId)) reviewedTodos = new Map(todos).set(todoId, prospectiveTodo); } outcome.review = {basis_digest: reviewBasis(outcome, reviewedTodos), items}; } diff --git a/skills/loopx-project/references/outcome-refinement.md b/skills/loopx-project/references/outcome-refinement.md index 9af89712e1..0b72195967 100644 --- a/skills/loopx-project/references/outcome-refinement.md +++ b/skills/loopx-project/references/outcome-refinement.md @@ -141,8 +141,11 @@ For changed requirements or reviews, add this envelope to the **existing** its ordinary routing, Turn, checkpoint and delivery arguments. This does not replace them. -If that same writeback includes an explicit Explore result, a newly authored -complete review binds the exact additive result link expected from this batch. +If that same writeback includes an explicit Explore result for work linked to +the reviewed outcome, a newly authored complete review binds the exact additive +result link expected from this batch. A valid result for separate same-Agent +work leaves this outcome's basis and work links unchanged; both still use the +existing ownership and intent validation. It remains stale until the Todo owner commits the link; failed delivery replays the original digest. Other work/evidence changes still require review. Do not repeat an unchanged review solely to acknowledge a successfully delivered diff --git a/skills/loopx-self-repair/references/repair-patterns.md b/skills/loopx-self-repair/references/repair-patterns.md index 900bf74c94..02a65b952e 100644 --- a/skills/loopx-self-repair/references/repair-patterns.md +++ b/skills/loopx-self-repair/references/repair-patterns.md @@ -5,7 +5,7 @@ teaches a reusable control-plane lesson. | Pattern | Symptoms | Evidence To Read | Likely Root | Durable Repair | | --- | --- | --- | --- | --- | -| `outcome_review_same_batch_link_invalidation` | A successful review-plus-result refresh immediately reports its Agent review basis stale; an unchanged standalone review is then repeated. | Exact refinement and result-delivery receipts, original CAS revision, retained conclusions and canonical Todo links before/after; distinguish the authored effect from other new evidence. | Refinement hashes pre-link work, then post-writeback delivery adds a work-basis input. | Bind only the newly authored complete review to the exact normalized same-batch additive link using the existing typed append rule and original CAS snapshot. Keep it stale until actual commit. Preserve independent verification, all retained requirements, separate/legacy reviews, initial CAS refusal, failed delivery, exact receipt replay and concurrent work/evidence invalidation through real File/SQLite CLI. Never exclude all evidence links, refresh old review digests against an arbitrary later head, reorder effects before primary admission or claim model/score benefit from this repair. | +| `outcome_review_same_batch_link_invalidation` | A successful review-plus-result refresh immediately reports its Agent review basis stale; an unchanged standalone review is then repeated. | Exact refinement and result-delivery receipts, original CAS revision, retained conclusions and canonical Todo links before/after; distinguish the authored effect from other new evidence. | Refinement hashes pre-link work, then post-writeback delivery adds a work-basis input. | Validate the complete intent and same-Goal ownership before determining relevance in the typed owner. Bind only the newly authored complete review whose outcome includes that work to the exact normalized same-batch additive link using the shared append rule and original CAS snapshot. Keep it stale until actual commit; a valid separate-work result leaves the reviewed outcome unchanged. Preserve independent verification, all retained requirements, separate/legacy reviews, initial CAS refusal, failed delivery, exact receipt replay and concurrent work/evidence invalidation through real File/SQLite CLI. Never exclude all evidence links, refresh old review digests against an arbitrary later head, reorder effects before primary admission or claim model/score benefit from this repair. | | `answer_question_replaced_by_delivery_status` | A user asks why a result occurred or whether it is expected, but receives measurements, tests or PR progress even though the reply was delivered completely. | The actual question and relevant corrections, current receiver brief, shared answer guidance, and the complete message on its original return route. | The author answered implementation progress instead of the causal question; successful transport or correct counters were mistaken for a useful answer. | Retain the correction in the existing Task and outcome, then repair the existing shared generation guidance: explain meaning, supported mechanism, expectations and the specific unknown before supporting data. Keep hypotheses separate from confirmed causes and preserve the qualified investigation owner. Verify guidance at native callers and read back the actual explanatory answer. Do not add keyword-based answer scoring, a second model rewrite, another display or a duplicate investigation; a prompt test or delivery receipt does not prove model compliance or user adoption. | | `queue_action_type_coercion` | A queued input action such as `["edit"]` is accepted but applies cancellation, or a stored operation accepts a non-string action. | Original JSON types, the typed planner and receipt decoder, real HTTP responses, canonical Turn/transcript/events/Session snapshots and native IO before/after refusal. | Membership checks stringify the action while effect selection and replay compare its original literal value. | Validate the original literal action at the existing owner before branching and when decoding pending or committed receipts. Cover arrays, nested arrays, objects, nulls, numbers, booleans and unknown strings; require rejection without state or native effects, then verify legal edit/cancel, original-operation replay and queue races. Preserve malformed historical evidence rather than coercing or rewriting receipts. | | `collaboration_return_format_projection_gap` | A complete answer that fits the result budget is rejected, or a published versioned return becomes unreadable or cannot replay through the real CLI. | Selected renderer output and Unicode count; persisted reply, compact publication/Inbox observations, original-route delivery and legacy replay at the same boundary. | Validation counted a different renderer before choosing the immutable format, or a compact codec omitted the persisted format marker and silently treated it as legacy. | Apply the unchanged budget to the actual selected format, carry its existing marker through every compact observation, and retain old bytes on replay. Validate BMP/non-BMP exact-limit and one-character-over cases, unsupported versions, real CLI publication/retry, original-audience return and File/SQLite packaged readback. Do not raise the budget, truncate the answer, rewrite historical receipts or add a second model. | diff --git a/tests/control_plane/test_outcome_explore_review_basis_cli.py b/tests/control_plane/test_outcome_explore_review_basis_cli.py index dc6347ca54..f66f69a9a6 100644 --- a/tests/control_plane/test_outcome_explore_review_basis_cli.py +++ b/tests/control_plane/test_outcome_explore_review_basis_cli.py @@ -14,7 +14,7 @@ ) -def _batch_fixture(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, provider: str): +def _batch_fixture(tmp_path: Path, monkeypatch: pytest.MonkeyPatch, provider: str, *, separate_review: bool = False): isolate_sqlite_runtime(tmp_path, monkeypatch) project, runtime, registry = _write_fixture(tmp_path) config = json.loads(registry.read_text()) @@ -30,13 +30,17 @@ def call(*args: str, expected_code: int = 0) -> dict: def inspect() -> dict: return call("goal-acceptance", "inspect", "--goal-id", GOAL_ID) + reviewed_todo = (call("todo", "add", "--goal-id", GOAL_ID, "--role", "agent", + "--claimed-by", AGENT_ID, "--text", "Validate a separate retained outcome", + "--task-class", "advancement_task", "--action-kind", "validate")["todo_id"] + if separate_review else TODO_ID) requirements = [{"id": key, "description": description} for key, description in ( ("content", "Preserve the requested content"), ("recovery", "Validate interrupted recovery"), ("scope", "Keep conclusions within the tested scope"), )] declaration = {"outcome_id": "export", "owner_agent_id": AGENT_ID, - "objective": "Deliver a recoverable export", "todo_ids": [TODO_ID], + "objective": "Deliver a recoverable export", "todo_ids": [reviewed_todo], "requirements": requirements, "source": {"ref": "message:export", "summary": "The owner requires a recoverable export"}} document = tmp_path / "declaration.json" @@ -65,6 +69,31 @@ def inspect() -> dict: return project, runtime, registry, call, inspect, declaration, reviews, batch +@pytest.mark.parametrize("provider", ["file", "sqlite"]) +def test_batch_review_of_separate_outcome_retains_its_own_work_basis( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, provider: str, +) -> None: + _, _, _, call, inspect, declaration, reviews, batch = _batch_fixture( + tmp_path, monkeypatch, provider, separate_review=True) + before = inspect() + preview = call(*batch, "--dry-run") + assert preview["goal_outcome_update"]["status"] == "planned" and inspect() == before + result = call(*batch) + assert result["explore_result_delivery"]["ok"] + outcome = inspect()["goal_acceptance_contract"]["outcome_requirements"]["outcomes"][0] + assert outcome["revision"] == 2 and outcome["todo_ids"] == declaration["todo_ids"] + assert outcome["todo_ids"] != [TODO_ID], "batching does not attach unrelated work to the reviewed outcome" + for row, review in zip(outcome["requirements"], reviews, strict=True): + assert row["agent_review"] == {**review, "basis_current": True, "independently_verified": False} + current = call("todo", "list", "--goal-id", GOAL_ID, "--todo-id", TODO_ID)["todo"] + other = call("todo", "list", "--goal-id", GOAL_ID, "--todo-id", outcome["todo_ids"][0])["todo"] + assert current["explore_result_node_refs"] == [ATTACHMENT["node_id"]] + assert not other.get("explore_result_node_refs") + retained = inspect() + replay = call(*batch) + assert replay["goal_outcome_update"]["status"] == "replayed" and inspect() == retained + + @pytest.mark.parametrize("provider", ["file", "sqlite"]) @pytest.mark.parametrize("source_mode", ["file", "path_delta"]) def test_batch_review_is_bound_to_exact_authored_link_without_new_review( diff --git a/tests/control_plane_ts/goal_outcome_review_work_basis.test.ts b/tests/control_plane_ts/goal_outcome_review_work_basis.test.ts index e2259cb027..23335d1371 100644 --- a/tests/control_plane_ts/goal_outcome_review_work_basis.test.ts +++ b/tests/control_plane_ts/goal_outcome_review_work_basis.test.ts @@ -76,10 +76,10 @@ test("prospective review covers only the exact additive effect and never mutates assert.equal(current(linked(ordinary)), false, "separately reviewed state must not adopt a later link"); }); -test("reviewed updates cannot rebind old reviews, replace work or act on unlinked/foreign work", () => { +test("reviewed updates cannot rebind old reviews, replace work or act on foreign work", () => { const head = fixture(); for (const invalid of [ - {...update, status: "done"}, {...update, todo_id: "todo_unrelated"}, + {...update, status: "done"}, {...update, todo_id: "todo_missing"}, {...update, append_explore_result_node_refs: []}, {...update, append_explore_result_node_refs: [node, "question_other"]}, {...update, append_explore_result_node_refs: ["/local/path"]}, @@ -91,6 +91,27 @@ test("reviewed updates cannot rebind old reviews, replace work or act on unlinke assert.throws(() => refineGoalOutcome(foreign, goal, null, "author", review, update)); }); +test("unrelated batch work is validated without changing the reviewed outcome's basis", () => { + const head = fixture(), other = {...update, todo_id: "todo_unrelated"}; + const reviewed = {...head, goal_outcomes: refineGoalOutcome(head, goal, null, "author", review, other)}; + assert.equal(current(reviewed), true, "an unrelated pending effect does not make this outcome stale"); + const delivered = withTodos(reviewed, row => row.todo_id === other.todo_id + ? {...row, ...appendTodoExploreReferences(row, [node])} : row); + assert.equal(current(delivered), true); + assert.deepEqual(reviewed.goal_outcomes.outcomes[0].todo_ids, declaration.todo_ids); + for (const invalid of [ + {...other, status: "done"}, {...other, append_explore_result_node_refs: []}, + {...other, append_explore_result_node_refs: [node, "question_other"]}, + {...other, append_explore_result_node_refs: ["/local/path"]}, + ]) assert.throws(() => refineGoalOutcome(head, goal, null, "author", review, invalid)); + const foreign = withTodos(head, row => row.todo_id === other.todo_id ? {...row, claimed_by: "other"} : row); + assert.throws(() => refineGoalOutcome(foreign, goal, null, "author", review, other)); + assert.throws(() => refineGoalOutcome(head, goal, null, null, review, other)); + assert.throws(() => refineGoalOutcome(head, goal, null, "author", {outcome_id: "export"}, other)); + assert.equal(current(withTodos(delivered, row => row.todo_id === todoId + ? {...row, text: "Changed reviewed work"} : row)), false); +}); + for (const provider of ["file", "sqlite"] as const) test(`${provider}: exact CAS and receipts retain anticipated basis across restart and lost replies`, async t => { const root = await mkdtemp(join(tmpdir(), "outcome-reviewed-work-")); t.after(() => rm(root, {recursive: true, force: true}));