feat(drone): 3-layer timeout policy — per-command overrides + --timeout flag + 30s default, override hint in error (DPLAN-0245). 878 tests green
This commit is contained in:
@@ -9,6 +9,27 @@ PyPI version — not the changelog header.
|
||||
|
||||
---
|
||||
|
||||
## [2026-07-16]
|
||||
|
||||
### Added
|
||||
|
||||
- **Close pipeline completes itself (DPLAN-0245): auto-vectorization +
|
||||
crash-safe registry writes + drone timeout policy.** Closing a plan now
|
||||
produces all side effects from one command — `post_close_runner` invokes
|
||||
@memory's plan intake directly after archival (detached, loud on failure,
|
||||
drains any backlog it finds), so plans can no longer silently pile up
|
||||
unvectorized. Plan registry saves (@flow `save_registry` + mbank
|
||||
`save_flow_registry`) now use the O_EXCL lockfile + atomic
|
||||
tempfile-and-replace pattern, closing the same lost-update race class fixed
|
||||
earlier in CLOSED_PLANS. @drone gained a 3-layer timeout policy: per-command
|
||||
overrides (`@memory process-plans` 120s, `@flow close` 90s), a `--timeout N`
|
||||
flag, 30s default — replacing the flat 30s guillotine that killed legitimate
|
||||
long commands mid-pipeline; the timeout error now says how to override.
|
||||
Proven end-to-end live: one `drone @flow close` on a throwaway plan yielded
|
||||
archive + vectors + ledger + registry with zero manual steps, and the
|
||||
auto-trigger swept a pre-existing backlog file on its first run. 730 flow +
|
||||
878 drone tests green, seedgo 100%.
|
||||
|
||||
## [2026-07-15]
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -51,6 +51,21 @@ INTERACTIVE_COMMANDS = ("monitor", "audit", "watchdog", "status")
|
||||
INTERACTIVE_BRANCHES = ("cli", "backup")
|
||||
|
||||
|
||||
def _extract_timeout(args: list[str]) -> tuple[list[str], int | None]:
|
||||
"""Extract --timeout N from an arg list. Returns (cleaned_args, timeout_or_None)."""
|
||||
if "--timeout" not in args:
|
||||
return args, None
|
||||
idx = args.index("--timeout")
|
||||
if idx + 1 >= len(args):
|
||||
return args, None
|
||||
try:
|
||||
timeout = int(args[idx + 1])
|
||||
except ValueError:
|
||||
logger.info("--timeout value %r is not an integer, ignoring", args[idx + 1])
|
||||
return args, None
|
||||
return args[:idx] + args[idx + 2 :], timeout
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# AUTO-DISCOVERY
|
||||
# =============================================================================
|
||||
@@ -92,6 +107,7 @@ def show_help() -> None:
|
||||
table.add_row("list", "List registered custom commands")
|
||||
table.add_row("remove <name>", "Remove a custom command")
|
||||
table.add_row("rm <path> [<path>...]", "Contained safe-delete (project + tmp)")
|
||||
table.add_row("--timeout <seconds>", "Override subprocess timeout (default 30s)")
|
||||
table.add_row("--help", "Show this help")
|
||||
table.add_row("--version", "Show version")
|
||||
|
||||
@@ -340,6 +356,7 @@ def _handle_custom_command(args: list[str]) -> int:
|
||||
target = cmd_data["target"]
|
||||
command = cmd_data["command"]
|
||||
cmd_args = list(cmd_data.get("args", [])) + remaining_args
|
||||
cmd_args, explicit_timeout = _extract_timeout(cmd_args)
|
||||
module_name = target.lstrip("@").lower()
|
||||
|
||||
interactive = command in INTERACTIVE_COMMANDS or module_name in INTERACTIVE_BRANCHES
|
||||
@@ -349,6 +366,7 @@ def _handle_custom_command(args: list[str]) -> int:
|
||||
target,
|
||||
command,
|
||||
args=cmd_args if cmd_args else None,
|
||||
timeout=explicit_timeout,
|
||||
interactive=interactive,
|
||||
)
|
||||
except (BranchNotFoundError, CommandExecutionError, RegistryError) as exc:
|
||||
@@ -412,6 +430,7 @@ def _handle_target(args: List[str]) -> int:
|
||||
"""Handle `drone @target command [args]` or `drone @target --help`."""
|
||||
target = args[0]
|
||||
rest = args[1:]
|
||||
rest, explicit_timeout = _extract_timeout(rest)
|
||||
module_name = target.lstrip("@").lower()
|
||||
|
||||
first_cmd = rest[0] if rest and rest[0] not in ("--help", "-h") else None
|
||||
@@ -470,6 +489,7 @@ def _handle_target(args: List[str]) -> int:
|
||||
target,
|
||||
command,
|
||||
args=cmd_args if cmd_args else None,
|
||||
timeout=explicit_timeout,
|
||||
interactive=interactive,
|
||||
)
|
||||
except (BranchNotFoundError, CommandExecutionError, RegistryError) as exc:
|
||||
|
||||
@@ -21,6 +21,29 @@ from .exceptions import CommandExecutionError
|
||||
from aipass.drone.apps.handlers.json import json_handler
|
||||
|
||||
|
||||
DEFAULT_TIMEOUT = 30
|
||||
|
||||
TIMEOUT_OVERRIDES: dict[str, dict[str, int]] = {
|
||||
"memory": {"process-plans": 120},
|
||||
"flow": {"close": 90},
|
||||
}
|
||||
|
||||
|
||||
def resolve_timeout(branch: str, command: str | None, explicit: int | None = None) -> int:
|
||||
"""Resolve subprocess timeout for a branch command.
|
||||
|
||||
Priority: explicit flag > per-command policy > DEFAULT_TIMEOUT.
|
||||
"""
|
||||
if explicit is not None:
|
||||
return explicit
|
||||
branch_key = branch.lstrip("@").lower()
|
||||
if command and branch_key in TIMEOUT_OVERRIDES:
|
||||
cmd_timeout = TIMEOUT_OVERRIDES[branch_key].get(command)
|
||||
if cmd_timeout is not None:
|
||||
return cmd_timeout
|
||||
return DEFAULT_TIMEOUT
|
||||
|
||||
|
||||
@dataclass
|
||||
class CommandResult:
|
||||
"""Result of a routed command execution."""
|
||||
@@ -82,7 +105,10 @@ def execute_command(
|
||||
return CommandResult(stdout="", stderr="", exit_code=130, branch="", command="")
|
||||
raise
|
||||
except subprocess.TimeoutExpired as e:
|
||||
raise CommandExecutionError(f"Command timed out after {timeout}s: {' '.join(full_cmd)}") from e
|
||||
raise CommandExecutionError(
|
||||
f"Command timed out after {timeout}s: {' '.join(full_cmd)}\n"
|
||||
f" Override with: drone @<target> <command> --timeout <seconds>"
|
||||
) from e
|
||||
except FileNotFoundError as e:
|
||||
raise CommandExecutionError(f"Executable not found: {executable!r}") from e
|
||||
except OSError as e:
|
||||
|
||||
@@ -20,7 +20,7 @@ from typing import Dict, List, Optional
|
||||
|
||||
from aipass.prax.apps.modules.logger import system_logger
|
||||
from aipass.cli.apps.modules import console
|
||||
from aipass.drone.apps.handlers.executor import CommandResult
|
||||
from aipass.drone.apps.handlers.executor import CommandResult, resolve_timeout
|
||||
from aipass.drone.apps.handlers.json import json_handler
|
||||
from aipass.drone.apps.handlers.router_handler import (
|
||||
detect_caller_branch_name,
|
||||
@@ -90,28 +90,38 @@ def route_command(
|
||||
target: str,
|
||||
command: Optional[str] = None,
|
||||
args: Optional[List[str]] = None,
|
||||
timeout: int = 30,
|
||||
timeout: int | None = None,
|
||||
interactive: bool = False,
|
||||
) -> CommandResult:
|
||||
"""Route a command to a branch's entry point.
|
||||
|
||||
Resolves @target to a path, then delegates to the handler for execution.
|
||||
When command is None, runs the branch with no args (introspection).
|
||||
|
||||
Timeout resolution: explicit value > per-command policy > DEFAULT_TIMEOUT.
|
||||
"""
|
||||
branch_path = resolve_branch(target)
|
||||
branch_name = target.lstrip("@").lower()
|
||||
resolved_timeout = resolve_timeout(branch_name, command, timeout)
|
||||
|
||||
caller = detect_caller_branch_name(Path.cwd())
|
||||
if not caller:
|
||||
caller = os.environ.get("AIPASS_BRANCH_NAME")
|
||||
caller_tag = f" [CALLER:{caller.upper()}]" if caller else ""
|
||||
logger.info("Routing @%s%s → %s %s", branch_name, caller_tag, command or "(introspection)", args or [])
|
||||
logger.info(
|
||||
"Routing @%s%s → %s %s (timeout=%ds)",
|
||||
branch_name,
|
||||
caller_tag,
|
||||
command or "(introspection)",
|
||||
args or [],
|
||||
resolved_timeout,
|
||||
)
|
||||
return execute_branch_command(
|
||||
branch_path=branch_path,
|
||||
branch_name=branch_name,
|
||||
command=command,
|
||||
args=args,
|
||||
timeout=timeout,
|
||||
timeout=resolved_timeout,
|
||||
interactive=interactive,
|
||||
)
|
||||
|
||||
@@ -147,7 +157,7 @@ def print_introspection():
|
||||
def route_all(
|
||||
command: str,
|
||||
args: Optional[List[str]] = None,
|
||||
timeout: int = 30,
|
||||
timeout: int | None = None,
|
||||
) -> Dict[str, CommandResult]:
|
||||
"""Route the same command to ALL active branches in the registry."""
|
||||
if args is None:
|
||||
|
||||
@@ -355,6 +355,7 @@ class TestHandleCustomCommand:
|
||||
"@seedgo",
|
||||
"audit",
|
||||
args=["aipass"],
|
||||
timeout=None,
|
||||
interactive=True,
|
||||
)
|
||||
|
||||
@@ -380,6 +381,7 @@ class TestHandleCustomCommand:
|
||||
"@seedgo",
|
||||
"audit",
|
||||
args=["aipass", "@drone"],
|
||||
timeout=None,
|
||||
interactive=True,
|
||||
)
|
||||
|
||||
@@ -616,6 +618,7 @@ class TestMainIntegration:
|
||||
"@seedgo",
|
||||
"audit",
|
||||
args=["aipass"],
|
||||
timeout=None,
|
||||
interactive=True,
|
||||
)
|
||||
|
||||
@@ -642,6 +645,7 @@ class TestMainIntegration:
|
||||
"@seedgo",
|
||||
"audit",
|
||||
args=["aipass", "@drone"],
|
||||
timeout=None,
|
||||
interactive=True,
|
||||
)
|
||||
|
||||
@@ -714,5 +718,6 @@ class TestMatchCommandIntegration:
|
||||
"@flow",
|
||||
"create",
|
||||
args=["--type=plan", "my-plan"],
|
||||
timeout=None,
|
||||
interactive=False,
|
||||
)
|
||||
|
||||
@@ -903,3 +903,71 @@ class TestAipassIntercept:
|
||||
):
|
||||
result = main()
|
||||
assert result == 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _extract_timeout — --timeout flag parsing
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestExtractTimeout:
|
||||
"""Tests for --timeout flag extraction from arg lists."""
|
||||
|
||||
def test_no_flag(self) -> None:
|
||||
"""Args without --timeout pass through unchanged."""
|
||||
from aipass.drone.apps.drone import _extract_timeout
|
||||
|
||||
args = ["close", "FPLAN-0313"]
|
||||
cleaned, timeout = _extract_timeout(args)
|
||||
assert cleaned == ["close", "FPLAN-0313"]
|
||||
assert timeout is None
|
||||
|
||||
def test_flag_at_end(self) -> None:
|
||||
"""--timeout N at end of args is extracted."""
|
||||
from aipass.drone.apps.drone import _extract_timeout
|
||||
|
||||
cleaned, timeout = _extract_timeout(["process-plans", "--timeout", "120"])
|
||||
assert cleaned == ["process-plans"]
|
||||
assert timeout == 120
|
||||
|
||||
def test_flag_at_start(self) -> None:
|
||||
"""--timeout N at start of args is extracted."""
|
||||
from aipass.drone.apps.drone import _extract_timeout
|
||||
|
||||
cleaned, timeout = _extract_timeout(["--timeout", "90", "close", "FPLAN-0313"])
|
||||
assert cleaned == ["close", "FPLAN-0313"]
|
||||
assert timeout == 90
|
||||
|
||||
def test_flag_in_middle(self) -> None:
|
||||
"""--timeout N in the middle of args is extracted."""
|
||||
from aipass.drone.apps.drone import _extract_timeout
|
||||
|
||||
cleaned, timeout = _extract_timeout(["close", "--timeout", "60", "FPLAN-0313"])
|
||||
assert cleaned == ["close", "FPLAN-0313"]
|
||||
assert timeout == 60
|
||||
|
||||
def test_flag_without_value(self) -> None:
|
||||
"""--timeout at end with no value returns None and leaves args."""
|
||||
from aipass.drone.apps.drone import _extract_timeout
|
||||
|
||||
args = ["close", "--timeout"]
|
||||
cleaned, timeout = _extract_timeout(args)
|
||||
assert cleaned == args
|
||||
assert timeout is None
|
||||
|
||||
def test_flag_non_integer_value(self) -> None:
|
||||
"""--timeout with non-integer value returns None and leaves args."""
|
||||
from aipass.drone.apps.drone import _extract_timeout
|
||||
|
||||
args = ["close", "--timeout", "abc"]
|
||||
cleaned, timeout = _extract_timeout(args)
|
||||
assert cleaned == args
|
||||
assert timeout is None
|
||||
|
||||
def test_empty_args(self) -> None:
|
||||
"""Empty arg list returns empty with None timeout."""
|
||||
from aipass.drone.apps.drone import _extract_timeout
|
||||
|
||||
cleaned, timeout = _extract_timeout([])
|
||||
assert cleaned == []
|
||||
assert timeout is None
|
||||
|
||||
@@ -8,7 +8,12 @@ from unittest.mock import patch
|
||||
import pytest
|
||||
|
||||
from aipass.drone.apps.handlers.exceptions import CommandExecutionError
|
||||
from aipass.drone.apps.handlers.executor import execute_command
|
||||
from aipass.drone.apps.handlers.executor import (
|
||||
DEFAULT_TIMEOUT,
|
||||
TIMEOUT_OVERRIDES,
|
||||
execute_command,
|
||||
resolve_timeout,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -389,3 +394,63 @@ class TestShellSecurity:
|
||||
# The semicolon is treated as literal text, not a shell separator
|
||||
assert result.stdout.strip() == "hello; echo pwned"
|
||||
assert result.exit_code == 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 11. resolve_timeout — policy resolution
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestResolveTimeout:
|
||||
"""Timeout resolution: explicit > policy > default."""
|
||||
|
||||
def test_default_timeout(self):
|
||||
"""Unknown branch+command returns DEFAULT_TIMEOUT."""
|
||||
assert resolve_timeout("unknown", "whatever") == DEFAULT_TIMEOUT
|
||||
|
||||
def test_policy_override(self):
|
||||
"""Known branch+command returns the policy value."""
|
||||
for branch, cmds in TIMEOUT_OVERRIDES.items():
|
||||
for cmd, expected in cmds.items():
|
||||
assert resolve_timeout(branch, cmd) == expected
|
||||
|
||||
def test_explicit_wins_over_policy(self):
|
||||
"""Explicit timeout overrides the policy map."""
|
||||
branch = next(iter(TIMEOUT_OVERRIDES))
|
||||
cmd = next(iter(TIMEOUT_OVERRIDES[branch]))
|
||||
assert resolve_timeout(branch, cmd, explicit=999) == 999
|
||||
|
||||
def test_explicit_wins_over_default(self):
|
||||
"""Explicit timeout overrides the default."""
|
||||
assert resolve_timeout("unknown", "whatever", explicit=42) == 42
|
||||
|
||||
def test_none_command_returns_default(self):
|
||||
"""None command (introspection) returns default."""
|
||||
assert resolve_timeout("memory", None) == DEFAULT_TIMEOUT
|
||||
|
||||
def test_at_prefix_stripped(self):
|
||||
"""Leading @ on branch name is stripped before lookup."""
|
||||
for branch in TIMEOUT_OVERRIDES:
|
||||
cmd = next(iter(TIMEOUT_OVERRIDES[branch]))
|
||||
expected = TIMEOUT_OVERRIDES[branch][cmd]
|
||||
assert resolve_timeout(f"@{branch}", cmd) == expected
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# 12. Timeout error message includes --timeout hint
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestTimeoutErrorMessage:
|
||||
"""Timeout error tells the caller how to override."""
|
||||
|
||||
def test_timeout_error_includes_override_hint(self, temp_test_dir: Path):
|
||||
"""The timeout error message mentions --timeout."""
|
||||
with pytest.raises(CommandExecutionError, match="--timeout") as exc_info:
|
||||
execute_command(
|
||||
sys.executable,
|
||||
["-c", "import time; time.sleep(10)"],
|
||||
cwd=str(temp_test_dir),
|
||||
timeout=1,
|
||||
)
|
||||
assert "--timeout" in str(exc_info.value)
|
||||
|
||||
Reference in New Issue
Block a user