From 1212ccc10c57e365826e0c9f378f203ec67005ea Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Sun, 1 Nov 2026 12:58:12 +0800 Subject: [PATCH] fix(todo): offer lawful recovery for deferred wait edits Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- docs/reference/handoff-mode.md | 14 +++ .../coordination/task_lease_proof.ts | 55 ++++++++---- .../test_todo_administrative_journey.py | 87 +++++++++++++++++++ .../deferred_hard_lease_lifecycle.test.ts | 19 ++++ .../todo_administrative_edits.test.ts | 31 +++++++ 5 files changed, 187 insertions(+), 19 deletions(-) diff --git a/docs/reference/handoff-mode.md b/docs/reference/handoff-mode.md index da1007e9bd..88e5d2f365 100644 --- a/docs/reference/handoff-mode.md +++ b/docs/reference/handoff-mode.md @@ -284,9 +284,17 @@ under a fresh operation and current revision. Other execution fences remain: | Expired execution matches the retained claim; eligible registered replacement actor | Use ordinary `todo claim` for atomic takeover, then use its new reference. | | Active foreign holder, divergent history, or absent claim | Reconcile ownership through its lifecycle; never borrow another holder's proof. | | `soft_claim` with released same-owner history, ordinary owner copy edit | Omit lease proof; keep the history. No replacement lease is needed or allowed. | +| Rejected wait revision on an already deferred Todo | Review and explicitly resume the original Todo, then reapply the reviewed deferral using a separate operation and current provider revision. Neither step grants execution. | | Non-open Todo, acceptance hold or conflicting write scopes | Resolve the reported acquisition blocker. No acquire action is offered. | | Edit changes retained leased work requirements or status | Use the owning lifecycle transition; acquiring another lease cannot authorize the metadata edit. | +For a deferred wait revision, the returned retry uses `--status open +--clear-resume-when` with a reviewed reason, fresh update operation and current +provider CAS. Read that result before submitting the intended deferral with +another operation and fresh CAS. Do not repeat the rejected same-status request +or carry its resume condition into the resume command. An active retained lease +must be released by its holder first; explicit stale proof remains rejected. + The existing `open -> blocked -> open` lifecycle also accepts a typed prerequisite wait: use `todo update --status blocked --resume-when todo_done: --reason ''` after the active holder @@ -346,6 +354,12 @@ claim+lease 均不可执行，必须先审核迁移；合并式 claim+lease 仍 提示不包含 execution key、不授予权限，也不修改 mode 或 provider。并发造成 version 变化时重新读取，不能绕过 CAS；dry-run 和拒绝路径不产生 Todo 或租约写入。 +修改已 deferred Todo 的等待条件若被拒绝，恢复命令先以 `--status open +--clear-resume-when`、审核后的 reason、新 update operation 和当前 provider CAS +显式恢复原任务；读回后，再用另一个 operation 和新的 CAS 提交审核后的延期。 +不重复原同状态请求，也不把其 resume 条件带入恢复命令。两步均不授予执行权，完整 +任务和历史保留；有效的历史 lease 须先由 holder 释放，显式过期 proof 仍被拒绝。 + ### Agent 普通 CLI 操作 CLI 由明确的 `--agent-id` 认领给自己时，会在同一 canonical 事务中准备执行； diff --git a/loopx/control_plane/coordination/task_lease_proof.ts b/loopx/control_plane/coordination/task_lease_proof.ts index b0b4787bce..d8d836ebd0 100644 --- a/loopx/control_plane/coordination/task_lease_proof.ts +++ b/loopx/control_plane/coordination/task_lease_proof.ts @@ -181,6 +181,41 @@ export function todoUpdateLeaseRecovery(head: JsonObject, input: CoordinationTod if (lease?.active && !sameOwner) { return {...base, action: "reconcile_lease_owner", reason: "Another active holder owns this execution. Its owner must release or explicitly hand it over through the lease lifecycle; this request cannot borrow its proof or take over. Inspect the current holder before retrying."}; } + // Both a bundled edit and a wait revision may need the same administrative + // resume. Probe its existing owner once; this never changes admission. + const reopen = {...input, patch: {}, clear_fields: [], planning_intent: { + status: "open", reason: "Reviewed lifecycle recovery", clear_resume_when: true, + }}; + const lifecycleFacts = {goal_id: input.goal_id, todo_id: input.todo_id, + actor_agent_id: input.actor_agent_id, registered_agents: input.registered_agents, + lease: lease === null ? undefined : index.leases.get(input.todo_id), + lease_idempotency_key: input.lease_idempotency_key ?? null, + lease_expected_version: input.lease_expected_version ?? null, now: input.now}; + const canReopen = mode === "hard_lease" && isBlockedLifecycleTransition(reopen, todo) && + blockedLifecycleRejection(lifecycleFacts) === null; + const canResume = (mode === "hard_lease" || lease !== null) && isDeferredReopen(reopen, todo) && + deferredReopenRejection(lifecycleFacts) === null; + const reopenRetry = {command: "loopx todo update --status open --reason ''", + cli_args: ["todo", "update", "--goal-id", input.goal_id, "--todo-id", input.todo_id, + ...(input.actor_agent_id ? ["--agent-id", input.actor_agent_id] : []), "--status", "open"], + goal_id: input.goal_id, todo_id: input.todo_id, agent_id: input.actor_agent_id, + requires_flags: ["--reason"], + remove_flags: ["--text", "--claimed-by", "--required-write-scope", "--successor-todo-id"], + proof_source: "fresh_lifecycle_admission"}; + if (todo.status === "deferred" && intent.status === "deferred") { + if (!canResume) { + return {...base, action: "resolve_lifecycle_edit", + reason: "This deferred Todo cannot currently resume through its administrative lifecycle. Inspect the retained execution and wait before revising it; an active lease must be released by its holder first. Do not repeat the deferred update or acquire execution on suspended work.", + ...(lease?.active && sameOwner ? {release: {command: "loopx task-lease release", + requires_flags: ["--owner", "--idempotency-key", "--expected-version"], + proof_source: "current_owner_lease_readback"}} : {})}; + } + return {...base, action: "resolve_lifecycle_edit", + reason: "Review the existing wait, then explicitly resume the unchanged Todo before reapplying the reviewed deferral. Use a separate update operation and current provider revision for each step; remove the old resume condition from the resume request. No acquire or old execution proof is needed, and neither transition grants execution. Keep the original requirements, ownership and history.", + retry: {...reopenRetry, cli_args: [...reopenRetry.cli_args, "--clear-resume-when"], + requires_flags: ["--reason", "--update-operation-id", "--update-expected-provider-revision"], + remove_flags: [...reopenRetry.remove_flags, "--resume-when"]}}; + } if (intent.status === "blocked" || intent.status === "deferred") { return {...base, action: "resolve_lifecycle_edit", reason: "Pause with a reason and optional note/evidence in one Todo update. The current holder's lease is retired atomically; no acquire or separate release is needed. Work requirements, text, ownership and successor changes are separate operations. A foreign active holder cannot be retired by this request; explicit stale proof is never replaced.", @@ -221,31 +256,13 @@ export function todoUpdateLeaseRecovery(head: JsonObject, input: CoordinationTod // claim/acquire a lease that the blocked status itself makes ineligible. // This is a diagnostic probe only; the actual retry rechecks owner admission // and the lifecycle fence in the ordinary provider transaction. - const reopen = {...input, patch: {}, clear_fields: [], planning_intent: { - status: "open", reason: "Reviewed lifecycle recovery", clear_resume_when: true, - }}; - const lifecycleFacts = {goal_id: input.goal_id, todo_id: input.todo_id, - actor_agent_id: input.actor_agent_id, registered_agents: input.registered_agents, - lease: lease === null ? undefined : index.leases.get(input.todo_id), - lease_idempotency_key: input.lease_idempotency_key ?? null, - lease_expected_version: input.lease_expected_version ?? null, now: input.now}; - const canReopen = mode === "hard_lease" && isBlockedLifecycleTransition(reopen, todo) && - blockedLifecycleRejection(lifecycleFacts) === null; - const canResume = (mode === "hard_lease" || lease !== null) && isDeferredReopen(reopen, todo) && - deferredReopenRejection(lifecycleFacts) === null; if (intent.status === "open" && (canReopen || canResume)) { const nextEdit = mode === "soft_claim" ? "Then retry only a permitted owner copy edit without either lease proof flag. Planning, ownership and terminal edits keep their own admission; do not acquire a lease in soft_claim." : "Ordinary owner metadata then needs no execution lease; other work still uses its execution or lifecycle admission."; return {...base, action: "resolve_lifecycle_edit", reason: `Reopen this ${todo.status} Todo with status=open and a reviewed reason; optional note/evidence can commit with it. Keep text, work requirements, ownership and successors separate. Use a fresh update operation and current provider revision after reviewing the wait. An omitted wait is cleared by this explicit resume. ${nextEdit} Reopening alone grants no execution authority.`, - retry: {command: "loopx todo update --status open --reason ''", - cli_args: ["todo", "update", "--goal-id", input.goal_id, "--todo-id", input.todo_id, - ...(input.actor_agent_id ? ["--agent-id", input.actor_agent_id] : []), "--status", "open"], - goal_id: input.goal_id, todo_id: input.todo_id, agent_id: input.actor_agent_id, - requires_flags: ["--reason"], - remove_flags: ["--text", "--claimed-by", "--required-write-scope", "--successor-todo-id"], - proof_source: "fresh_lifecycle_admission"}}; + retry: reopenRetry}; } const editRejection = lease === null || isOwnerDeferral(input, todo) ? null : leasedTodoEditRejection(todo, intent); if (editRejection !== null) { diff --git a/tests/control_plane/test_todo_administrative_journey.py b/tests/control_plane/test_todo_administrative_journey.py index 6f344f216e..e293d4e56a 100644 --- a/tests/control_plane/test_todo_administrative_journey.py +++ b/tests/control_plane/test_todo_administrative_journey.py @@ -10,6 +10,7 @@ from canonical_authority_fixture import isolate_sqlite_runtime, promoted_create_fixture from loopx.chat_action_store import ChatActionStore from loopx.chat_actions import ChatActionService +from loopx.control_plane.coordination.local_authority import read_canonical_todos_if_promoted from loopx.control_plane.todos.mutation_api import add_goal_todo @@ -78,6 +79,92 @@ def test_cli_copy_pause_resume_defer_without_lease_bookkeeping(journey): assert cli("todo", "list")["todo"] == readback +@pytest.mark.parametrize("retained", ["absent", "released"]) +def test_deferred_wait_edit_recovers_by_reviewed_resume_then_deferral(journey, retained): + cli, _, _, runtime = journey + text = "Original requirements\n" + "完整验收 🙂 " * 80 + "FINAL_ACCEPTANCE" + cli("todo", "update", "--agent-id", "agent-a", "--text", text, + "--note", "Preserve this context", "--evidence", "evidence:original", "--priority", "P2") + if retained == "released": + cli("task-lease", "acquire", "--owner", "agent-a", "--idempotency-key", "original") + cli("todo", "update", "--agent-id", "agent-a", "--status", "deferred", + "--reason", "Wait for the original event", "--resume-when", "resume_at:2099-01-01T00:00:00Z") + + def snapshot(): + return read_canonical_todos_if_promoted(runtime_root=runtime, goal_id="goal-a", include_leases=True) + + before = snapshot() + original = before["todos"][0] + wait_edit = ("todo", "update", "--agent-id", "agent-a", "--status", "deferred", + "--reason", "Reviewed the next observation", "--resume-when", "resume_at:2099-02-01T00:00:00Z") + # A diagnostic is not permission to rewrite a suspended task or consume its + # released execution. Its proposed recovery must reach a lawful state. + rejected = cli(*wait_edit, ok=False) + assert snapshot() == before + recovery = rejected["recovery"] + assert recovery["execution_authority_granted"] is False + retry = recovery["retry"] + args = retry["cli_args"] + assert args[args.index("--status") + 1] == "open" + assert "--clear-resume-when" in args and "--resume-when" in retry["remove_flags"] + assert {"--reason", "--update-operation-id", "--update-expected-provider-revision"} <= set(retry["requires_flags"]) + assert "acquire" not in recovery + + wrong_owner = cli("todo", "update", "--agent-id", "agent-b", "--status", "open", + "--clear-resume-when", "--reason", "Cannot review another owner's wait", ok=False) + assert wrong_owner["error_code"] == "update_owner_mismatch" + assert "retry" not in wrong_owner["recovery"] + if retained == "released": + old = before["leases"][0] + refused = cli(*wait_edit, "--task-lease-idempotency-key", old["idempotency_key"], + "--task-lease-expected-version", str(old["version"]), ok=False) + assert refused["recovery"]["action"] == "inspect_current_proof" + assert "retry" not in refused["recovery"] + assert snapshot() == before + + def retry_cli(*extra): + child = subprocess.run([sys.executable, "-m", "loopx.cli", "--format", "json", *args, *extra], + capture_output=True, text=True, timeout=60) + assert child.returncode == 0, child.stdout + child.stderr + return json.loads(child.stdout) + + reviewed = ("--reason", "Owner reviewed the old wait before revising it", + "--update-operation-id", "resume-for-wait-edit", + "--update-expected-provider-revision", before["provider_revision"]) + assert retry_cli(*reviewed, "--dry-run")["status"] == "planned" + assert snapshot() == before + resumed = retry_cli(*reviewed) + assert resumed["deferred_resume_transition"]["execution_authority_granted"] is False + opened = snapshot() + assert opened["todos"][0]["status"] == "open" + assert opened["todos"][0].get("resume_when") is None + assert opened["leases"] == before["leases"] + assert retry_cli(*reviewed)["status"] == "replayed" + assert snapshot() == opened + + # Each new intent uses the current CAS; a historical recovery receipt cannot + # authorize the subsequent wait or revive the retired execution. + stale = cli(*wait_edit, "--update-operation-id", "stale-wait-edit", + "--update-expected-provider-revision", before["provider_revision"], ok=False) + assert stale["error_code"] == "provider_revision_mismatch" + assert snapshot() == opened + rearm = (*wait_edit, "--update-operation-id", "reviewed-wait-edit", + "--update-expected-provider-revision", opened["provider_revision"]) + applied = cli(*rearm) + assert applied["deferred_transition"]["execution_authority_granted"] is False + after = snapshot() + task = after["todos"][0] + assert task["status"] == "deferred" and task["resume_when"] == "resume_at:2099-02-01T00:00:00Z" + changed = {"status", "reason", "resume_when", "updated_at", "last_actor_agent_id"} + assert {k: v for k, v in original.items() if k not in changed} == { + k: v for k, v in task.items() if k not in changed} + assert after["leases"] == before["leases"] + assert cli(*rearm)["status"] == "replayed" + assert snapshot() == after + assert retry_cli(*reviewed)["status"] == "replayed" + assert snapshot() == after, "historical resume cannot replace the newer reviewed wait" + + def test_chat_uses_same_atomic_owner_and_refuses_changed_review_basis(journey, tmp_path): cli, todo_id, registry, runtime = journey cli("todo", "update", "--agent-id", "agent-a", "--task-repository", "git:github.com/example/original") diff --git a/tests/control_plane_ts/deferred_hard_lease_lifecycle.test.ts b/tests/control_plane_ts/deferred_hard_lease_lifecycle.test.ts index 5b7c4371e4..7b5ee17bde 100644 --- a/tests/control_plane_ts/deferred_hard_lease_lifecycle.test.ts +++ b/tests/control_plane_ts/deferred_hard_lease_lifecycle.test.ts @@ -130,6 +130,25 @@ for (const provider of ["file", "sqlite"] as const) { } }); + providerTest(`${provider}: wait-edit recovery cannot bypass an active holder`, async () => { + for (const holder of AGENTS) { + const store = await seeded(provider, {}, {...oldLease("active", "2026-09-06T00:00:00Z"), owner: holder}); + const before = await read(store); + const input = {...resume("revise-suspended-wait"), planning_intent: {status: "deferred", + resume_when: "resume_at:2026-09-06T01:00:00Z", reason: "Reviewed next observation"}}; + for (const dry_run of [true, false]) { + const rejected = await executeCoordinationTodoUpdate(store, {...input, dry_run}); + assert.equal(rejected.status, "failed", JSON.stringify(rejected)); + const recovery = rejected.recovery as JsonObject; + assert.equal(recovery.execution_authority_granted, false); + assert.equal(recovery.retry, undefined, "active execution cannot authorize administrative resume"); + assert.equal(recovery.acquire, undefined); + if (holder !== OWNER) assert.equal(recovery.action, "reconcile_lease_owner"); + assert.deepEqual(await read(store), before); + } + } + }); + providerTest(`${provider}: an unclaimed deferred Todo stays unclaimed; excluded actors cannot reopen it`, async () => { const unclaimed = await seeded(provider, {claimed_by: null}); const applied = await executeCoordinationTodoUpdate(unclaimed, resume("unclaimed-resume")); diff --git a/tests/control_plane_ts/todo_administrative_edits.test.ts b/tests/control_plane_ts/todo_administrative_edits.test.ts index 2a638837ef..eab6daf951 100644 --- a/tests/control_plane_ts/todo_administrative_edits.test.ts +++ b/tests/control_plane_ts/todo_administrative_edits.test.ts @@ -96,6 +96,37 @@ for (const provider of ["file", "sqlite", "postgresql"] as const) { assert.equal((resumed.blocked_lifecycle_transition as JsonObject).execution_authority_granted, false); assert.deepEqual((await head(store)).head.leases, paused.head.leases); }); + test(`${provider}: deferred wait recovery reaches a reviewed new wait without execution`, options, async t => { + const {store, input} = await fixture(t, provider, "released"); + const defer = {...input, operation_id: "original-wait", planning_intent: {status: "deferred", + resume_when: "resume_at:2026-09-26T06:00:00Z", reason: "Original observation"}}; + assert.equal((await executeCoordinationTodoUpdate(store, defer)).status, "applied"); + const before = await head(store); + const revised = {...defer, operation_id: "revised-wait", planning_intent: {...defer.planning_intent, + resume_when: "resume_at:2026-09-27T06:00:00Z", reason: "Reviewed next observation"}}; + const refused = await executeCoordinationTodoUpdate(store, revised); + assert.equal(refused.status, "failed"); + const recovery = refused.recovery as JsonObject; + assert.equal(recovery.execution_authority_granted, false); + const retry = recovery.retry as JsonObject, argv = retry.cli_args as string[]; + assert.equal(argv[argv.indexOf("--status") + 1], "open"); + assert.ok(argv.includes("--clear-resume-when")); + assert.deepEqual(await head(store), before); + const resume = {...input, operation_id: "reviewed-resume", expected_provider_revision: before.provider_revision, + planning_intent: {status: "open", clear_resume_when: true, reason: "Owner reviewed the old wait"}}; + assert.equal((await executeCoordinationTodoUpdate(store, resume)).status, "applied"); + const opened = await head(store); + const result = await executeCoordinationTodoUpdate(store, {...revised, expected_provider_revision: opened.provider_revision}); + assert.equal(result.status, "applied", JSON.stringify(result)); + assert.equal((result.deferred_transition as JsonObject).execution_authority_granted, false); + const after = await head(store), todo = (after.head.todos as JsonObject[])[0]!; + assert.equal(todo.status, "deferred"); + assert.equal(todo.resume_when, revised.planning_intent.resume_when); + assert.deepEqual(todo.required_write_scopes, ["src/**"]); + assert.deepEqual(after.head.leases, before.head.leases); + assert.equal((await executeCoordinationTodoUpdate(store, resume)).status, "replayed"); + assert.deepEqual(await head(store), after); + }); test(`${provider}: explicit stale proof, foreign holder and changed provider revision never auto-heal`, options, async t => { const {store, input} = await fixture(t, provider); const before = await head(store);