diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index de6a972d2e..50dc5e13d2 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -37,6 +37,7 @@ from ..progress_claims import MergedClaim, WorkingClaim from ..recorded_time import RecordedEventTime from ..standing_rules import HuntDue, RulesDue +from ..storage_rules import UnitStorage from ..tool_caller import ThreadCaller from ..tools import (CommsFireTool, CommsSendTool, CommsOrderCheckTool, CommsOrderServeTool, CommsOrderDoneTool, CommsOrderTool, @@ -487,10 +488,12 @@ def done(pid, sha, screenshot='', seen='', repo=''): if added := forwarders_added(diff, after): sys.exit(f'Refused: {sha} adds forwarders {added[:5]}; slop never counts as delivery. Order the hierarchy fix.') thread = COMMS.registry.require(unit) - if left := [str(t) for t in COMMS.registry.storage_rule.checkouts_of(thread) if pid.lower() in t.name.lower()]: - sys.exit(f'Refused: {unit} still has the checkout {left} for this order; storage rule: it removes it before the order counts as done.') + storage = UnitStorage.read(COMMS.registry.storage_rule, thread) + checkout = storage.rule.order_checkout(unit, pid) + if checkout in storage.extra_checkouts: + sys.exit(f'Refused: {unit} still has the checkout {checkout} for this order; storage rule: it removes it before the order counts as done.') try: - COMMS.registry.storage_rule.require_quota(thread) + storage.require_quota() except SelectedToolDenied as quota: sys.exit(f'Refused: {unit}: {quota}; storage rule: it cleans up before the order counts as done.') todo.order.run.require_artifact(seen or screenshot) diff --git a/src/agent_comms/storage_rules.py b/src/agent_comms/storage_rules.py index fa22024700..8ac352442d 100644 --- a/src/agent_comms/storage_rules.py +++ b/src/agent_comms/storage_rules.py @@ -178,8 +178,7 @@ def cleanup_due(self, free_gib: float) -> bool: return free_gib < self.cleanup_gib def require_quota(self, thread: Thread, *proposed: Path): - if UnitStorage.read(self, thread, *proposed).over_checkouts: - self.deny(f'at most {self.max_checkouts} checkouts besides the home checkout') + UnitStorage.read(self, thread, *proposed).require_quota() def require_allocation(self, thread: Thread, *proposed: Path): if self.below_floor(self.free_gib()): @@ -219,7 +218,7 @@ def order_text(self, unit: str, pythons: Sequence[str]) -> str: @dataclass(frozen=True) class UnitStorage(UndatedStandingRule): - """One unit's checkouts, derived on read from its registered worktrees; its rule judges them.""" + """The home storage slot (possibly non-Git) followed by extra Git checkouts.""" rule: StorageRule unit: str @@ -237,12 +236,20 @@ def violations(cls, comms: Comms, commander: str) -> tuple[UnitStorage, ...]: read = (cls.read(registry.storage_rule, registry.require(unit)) for unit in units) return tuple(storage for storage in read if storage.over_checkouts) + @property + def extra_checkouts(self) -> tuple[Path, ...]: + return self.checkouts[1:] + @property def over_checkouts(self) -> bool: - return len(self.checkouts) > self.rule.max_checkouts + 1 + return len(self.extra_checkouts) > self.rule.max_checkouts + + def require_quota(self) -> None: + if self.over_checkouts: + self.rule.deny(f'at most {self.rule.max_checkouts} checkouts besides the home checkout') def line(self) -> str: - return (f'VIOLATION storage quota: {self.unit} holds {len(self.checkouts) - 1} checkouts besides home, ' + return (f'VIOLATION storage quota: {self.unit} holds {len(self.extra_checkouts)} checkouts besides home, ' f'limit {self.rule.max_checkouts} -> order the cleanup') @@ -324,7 +331,7 @@ class RemoveCheckoutStorageEffect(StorageEffect): path: Path def check(self, rule, thread): - if self.path not in rule.checkouts_of(thread)[1:]: + if self.path not in UnitStorage.read(rule, thread).extra_checkouts: rule.deny('only a current extra checkout can be removed') diff --git a/tests/test_commander_problem_orders.py b/tests/test_commander_problem_orders.py index 2d7c880cdc..12b8553247 100644 --- a/tests/test_commander_problem_orders.py +++ b/tests/test_commander_problem_orders.py @@ -354,6 +354,37 @@ def strikes(delivery, at): assert strikes(OperationDelivery(), 9000) == () +@pytest.mark.parametrize('retained_problem', [None, 'P197', 'P1970']) +def test_done_distinguishes_non_git_home_from_exact_order_checkout( + commander, commander_script, tmp_path, retained_problem): + from agent_comms.storage_rules import StorageRule, UnitStorage + + comms, _ = commander + script, _ = commander_script + home = tmp_path / 'P197-application' + home.mkdir() + comms.registry.declare(comms.registry.new_thread('scratch', frozenset({'comms'}), str(home))) + rule = StorageRule(checkout_base=tmp_path / 'wt', scratch_base=tmp_path / 'scratch', floor_gib=0) + comms.registry.storage_rule = script.COMMS.registry.storage_rule = rule + if retained_problem: + repository = tmp_path / 'repository' + subprocess.run(('git', 'init', '-q', str(repository)), check=True) + subprocess.run(('git', '-C', str(repository), '-c', 'user.name=t', '-c', 'user.email=t@t', + 'commit', '-q', '--allow-empty', '-m', 'base'), check=True) + subprocess.run(('git', '-C', str(repository), 'worktree', 'add', '-q', '-b', 'work', + str(rule.order_checkout('scratch', retained_problem))), check=True) + todo = create(commander, 'P197', owner='scratch', deadline=time.time() + 3600) + assert not (home / '.git').exists() + assert UnitStorage.read(rule, comms.registry.require('scratch')).checkouts[0] == home + if retained_problem == 'P197': + with pytest.raises(SystemExit, match='still has the checkout'): + script.done('P197', 'op', seen=working(comms)[0].screenshot) + assert comms.goals.todos.get('P197').pending + else: + script.done('P197', 'op', seen=working(comms)[0].screenshot) + assert not comms.goals.todos.get('P197').pending + + def test_a_late_commit_done_through_the_tool_is_struck_in_its_delivery(commander): """Core strikes a late delivered commit in the revision that records it; no script is needed.""" comms, caller = commander