From 259739aa06874d4b07d04e2feaf8865f5eecaf31 Mon Sep 17 00:00:00 2001 From: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> Date: Sat, 10 Oct 2026 02:33:03 +0800 Subject: [PATCH] fix(cli): preserve option-like values in lease recovery argv Signed-off-by: LoopX Agent <337587101+loopx-agent@users.noreply.github.com> --- loopx/cli_commands/task_lease_arguments.py | 17 ++++--- .../test_task_lease_argument_recovery.py | 45 +++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/loopx/cli_commands/task_lease_arguments.py b/loopx/cli_commands/task_lease_arguments.py index c66cbc6285..4b2be0e295 100644 --- a/loopx/cli_commands/task_lease_arguments.py +++ b/loopx/cli_commands/task_lease_arguments.py @@ -27,6 +27,13 @@ def _present(value: object) -> bool: return value is not None and value is not False and value != "" and value != [] +def _option_args(flag: str, value: object) -> list[str]: + text = str(value) + # A legal value may look like an option. Bind it explicitly so argparse + # cannot reinterpret the repair's identity, route or repeated scope. + return [f"{flag}={text}"] if text.startswith("-") else [flag, text] + + class TaskLeaseArgumentError(ValueError): def __init__(self, *, missing: list[str], unsupported: list[str]) -> None: self.missing = missing @@ -40,12 +47,12 @@ def __init__(self, *, missing: list[str], unsupported: list[str]) -> None: def recovery(self, args: argparse.Namespace, *, registry_path: Path, runtime_root_arg: str | None) -> dict[str, object]: - cli_args = ["--registry", str(registry_path), "--format", "json"] + cli_args = [*_option_args("--registry", registry_path), "--format", "json"] if runtime_root_arg is not None: - cli_args.extend(["--runtime-root", runtime_root_arg]) + cli_args.extend(_option_args("--runtime-root", runtime_root_arg)) action = args.task_lease_command - cli_args.extend(["task-lease", action, "--goal-id", args.goal_id, - "--todo-id", args.todo_id]) + cli_args.extend(["task-lease", action, *_option_args("--goal-id", args.goal_id), + *_option_args("--todo-id", args.todo_id)]) for flag, field in LEASE_OPTION_FIELDS: value = getattr(args, field, None) if field not in LEASE_ACTION_FIELDS[action] or not _present(value): @@ -54,7 +61,7 @@ def recovery(self, args: argparse.Namespace, *, registry_path: Path, cli_args.append(flag) else: for item in value if isinstance(value, list) else [value]: - cli_args.extend([flag, str(item)]) + cli_args.extend(_option_args(flag, item)) return { "command": f"loopx task-lease {action}", "cli_args": cli_args, "requires_flags": self.missing, "remove_flags": self.unsupported, diff --git a/tests/control_plane/test_task_lease_argument_recovery.py b/tests/control_plane/test_task_lease_argument_recovery.py index 1fa2b479fd..b1912a13f3 100644 --- a/tests/control_plane/test_task_lease_argument_recovery.py +++ b/tests/control_plane/test_task_lease_argument_recovery.py @@ -104,6 +104,51 @@ def test_missing_inputs_are_combined_and_wrong_owner_is_not_repaired(lease_cli): assert run(action("inspect"))["lease"] == before +def test_recovery_round_trips_leading_hyphen_execution_keys(lease_cli): + run, action, _, _ = lease_cli + released = run(action("release", "--owner", "peer-a", "--idempotency-key", "original", + "--expected-version", "1")) + acquired = run(action("acquire", "--owner", "peer-a", "--idempotency-key=-execution", + "--write-scope", "src/**", "--expected-version", + str(released["lease"]["version"]))) + before = acquired["lease"] + for name, extra in [("renew", []), ("transfer", ["--new-owner", "peer-a", + "--new-idempotency-key=-receiver"])]: + rejected = run(action(name, "--owner", "peer-a", "--idempotency-key=-execution", + "--expected-version", str(before["version"]), + "--write-scope", "other/**", *extra), code=1) + assert run(action("inspect"))["lease"] == before + repaired = rejected["recovery"]["cli_args"] + applied = run(repaired)["lease"] + assert applied["version"] == before["version"] + 1 + assert applied["idempotency_key"] == ("-receiver" if name == "transfer" else "-execution") + assert applied["owner"] == "peer-a" and applied["write_scopes"] == ["src/**"] + assert run(repaired)["lease"] == applied + assert run(action("inspect"))["lease"] == applied + before = applied + + +def test_recovery_preserves_routing_and_repeatable_string_values(): + from loopx.cli import build_parser + from loopx.cli_commands.task_lease_arguments import validate_task_lease_arguments, TaskLeaseArgumentError + + parser = build_parser() + original = parser.parse_args([ + "--registry=-registry.json", "--runtime-root=-runtime", "task-lease", "acquire", + "--goal-id=-goal", "--todo-id=-todo", "--owner=-owner", "--idempotency-key=-key", + "--write-scope=-src/**", "--write-scope=docs with spaces/**", "--write-worktree=-tree", + "--new-idempotency-key=remove-me", "--expected-version", "0", + ]) + with pytest.raises(TaskLeaseArgumentError) as rejected: + validate_task_lease_arguments(original) + recovered = parser.parse_args(rejected.value.recovery( + original, registry_path=Path("-registry.json"), runtime_root_arg="-runtime")["cli_args"]) + for field in ("registry", "runtime_root", "goal_id", "todo_id", "owner", "idempotency_key", + "write_scopes", "write_worktree", "expected_version"): + assert getattr(recovered, field) == getattr(original, field) + assert recovered.new_idempotency_key is None + + def test_transfer_repair_retains_explicit_claim_handover(lease_cli, request): run, action, _, _ = lease_cli before = run(action("inspect"))["lease"]