diff --git a/docs/agents/behaviour-catalog.md b/docs/agents/behaviour-catalog.md index be48d160b6..055454b6f7 100644 --- a/docs/agents/behaviour-catalog.md +++ b/docs/agents/behaviour-catalog.md @@ -129,7 +129,7 @@ A done report that names its own measure instead of what its order's Check print Checked by: -- not enforced here: P197 (#1733) +- check in `agent_comms.todos.OrderDelivery.verified` Learned from: seed catalog pattern 4 (claude-ai message 1c8729ba0ed7); [Codex postmortem, failure 4](https://github.com/OpenHCSDev/toad/blob/main/docs/agents/CODEX-POSTMORTEM.md) diff --git a/src/agent_comms/behaviour_patterns.py b/src/agent_comms/behaviour_patterns.py index 6abd717c28..c9f402f136 100644 --- a/src/agent_comms/behaviour_patterns.py +++ b/src/agent_comms/behaviour_patterns.py @@ -19,7 +19,7 @@ from .declared_family import DeclaredFamily from .goals import OrderCheck from .standing_rules import BoardDisagrees, PlanIdle, StandingRule -from .todos import PlainText, StorageStrikeTodo, Todo, TodoStore, VoidStrikeTodo +from .todos import OrderDelivery, PlainText, StorageStrikeTodo, Todo, TodoStore, VoidStrikeTodo POSTMORTEM = "https://github.com/OpenHCSDev/toad/blob/main/docs/agents/CODEX-POSTMORTEM.md" COMMANDER_SKILL = "../../src/agent_comms/skills/comms-commander/SKILL.md" @@ -309,7 +309,7 @@ class WrongMeasurePattern(BehaviourPattern): signature = ( "A done report that names its own measure instead of what its order's Check printed." ) - checks = (PendingCheck("P197", "#1733"),) + checks = (CallableCheck(OrderDelivery.verified),) sources = (SeedCatalog(4), MeasuredWrongThingFailure()) diff --git a/src/agent_comms/child_process.py b/src/agent_comms/child_process.py index f12253f903..94a1958de5 100644 --- a/src/agent_comms/child_process.py +++ b/src/agent_comms/child_process.py @@ -1015,6 +1015,10 @@ def close(self) -> None: class CheckPrinted(DeclaredFamily, affix="Printed"): """The count printed by a child, or the absence of an integer.""" + @property + @abstractmethod + def line(self) -> str: ... + @staticmethod def decoded(stdout: bytes) -> CheckPrinted: counts = re.findall(r"\d+", stdout.decode(errors="replace")) @@ -1040,6 +1044,10 @@ def project[T](self, counted: Callable[[int], T], uncounted: Callable[[], T]) -> class CountPrinted(CheckPrinted): count: int + @property + def line(self) -> str: + return f"Check printed {self.count}" + def distance(self, expected: int) -> CheckPrinted: return CountPrinted(abs(self.count - expected)) @@ -1060,6 +1068,10 @@ def unproven(self, command: str, expected: int) -> tuple[str, ...]: @dataclass(frozen=True) class NoIntegerPrinted(CheckPrinted): + @property + def line(self) -> str: + return "Check printed no integer" + def distance(self, expected: int) -> CheckPrinted: return self diff --git a/src/agent_comms/commander/judge.py b/src/agent_comms/commander/judge.py index b26e77f4fb..24e4a6b27a 100644 --- a/src/agent_comms/commander/judge.py +++ b/src/agent_comms/commander/judge.py @@ -20,7 +20,7 @@ from dataclasses import asdict, dataclass from ..comms import Comms -from ..todos import AcceptedVerdict, PeerAcceptedVerdict, BluntText, OrderVerdict, SendBackVerdict, StrikeReason, Todo, Slop, Derivation, SupersededVerdict +from ..todos import AcceptedVerdict, PeerAcceptedVerdict, OrderVerdict, SendBackVerdict, Todo, Slop, Derivation, SupersededVerdict from ..tools import CommsUnjudgedTool, CommsVerdictTool from ..field_codec import FieldCodec from ..tool_caller import ThreadCaller @@ -160,8 +160,6 @@ def check_edits(self, edits: list[Edit]) -> None: ... def report(self, todos: tuple[Todo, ...]) -> None: ... def run(self) -> None: - if len(self.why.split()) < 6: - sys.exit('Refused: state which intent the edit serves or violates, and why (6+ words).') orders = {todo.id: todo for todo in order.held_orders() if not self.msg or todo.sent_in(self.msg)} edits = {edit.id: edit for edit in Edit.unjudged(order.COMMS, ME) if edit.problem in orders} ids = [token.split(':')[0] for token in self.ids] @@ -230,7 +228,7 @@ def report(self, todos: tuple[Todo, ...]) -> None: @dataclass(frozen=True, kw_only=True) -class BackVerb(VerdictVerb): +class BackVerb(order.CitedState, VerdictVerb): help = "send the edit back; the roast is posted to the verdict's audience" msg: str = option(help="the required order message") roast: str = option(help="blunt, why it is stupid, with because") @@ -248,16 +246,11 @@ def slop(self) -> Slop: def operation(self, todo: Todo, edit_id: str) -> SendBackVerdict: return SendBackVerdict(todo.revision, order.COMMS.registry.require(ME).incarnation, todo.held(), edit_id, - reason=StrikeReason(f'{todo.id} edit sent back: {self.why}')) + reason=f'{todo.id} edit sent back: {self.why}', + evidence=self.evidence()) def check_edits(self, edits: list[Edit]) -> None: - if len(self.roast.split()) < 12 or not re.search(r'\bbecause\b', self.roast): - sys.exit('Refused: --roast needs 12+ words with because, naming the duplicated fact and its owner') - if len(self.fix.split()) < 8: - sys.exit('Refused: --fix needs 8+ words naming the owner and replacing shape') self.slop() - order.refuse(self.roast, BluntText) - order.refuse(' '.join((self.why, self.roast, self.fix)), *order.OUTGOING) def report(self, todos: tuple[Todo, ...]) -> None: for todo in todos: diff --git a/src/agent_comms/commander/order.py b/src/agent_comms/commander/order.py index de6a972d2e..12f5db612e 100644 --- a/src/agent_comms/commander/order.py +++ b/src/agent_comms/commander/order.py @@ -34,7 +34,7 @@ from ..native_tool_call import SelectedToolDenied from ..messages import Message from ..plan_phases import OrderPhase, UnplannedPhase -from ..progress_claims import MergedClaim, WorkingClaim +from ..progress_claims import WorkingClaim from ..recorded_time import RecordedEventTime from ..standing_rules import HuntDue, RulesDue from ..tool_caller import ThreadCaller @@ -44,7 +44,7 @@ from ..todos import (ApplicationOrderRun, VerdictAudience, InstallDeadline, PlannedInstallDeadline, OrderMessage, MechanicalOrderRun, OrderRun, BluntText, CommitDelivery, NotifiedFirstCallEvent, ObservedFirstCallEvent, OperationDelivery, CleanHunt, OrderedHunt, PendingProblemOrder, RecordFirstCallTodo, OrderDeletion, StrikeReason, StrikeTodo, VoidStrikeTodo, LocatedProblemFinding, UnlocatedProblemFinding, FileDeliveryEvidence, NoDeliveryEvidence, - WithdrawTodo, AssignTodo, DirectiveText, OrderProgress, Todo, Slop) + WithdrawTodo, AssignTodo, DirectiveText, OrderProgress, Todo, Slop, Evidence) from . import capacity from .config import Commander, CommanderConfig, CommanderRole, Repo @@ -178,7 +178,7 @@ def forwarders_added(diff, sources=()): def send(unit, due, body: str, clean=False, mechanical=False, reassign=None, new=None, - traced=None, replaces=None, disk=False, finding=UnlocatedProblemFinding()): + traced=None, replaces=None, disk=False, finding=UnlocatedProblemFinding(), evidence=()): """Validate an order body and record it in Core: a new problem (new=(PID, TEXT)) or the listed open problems. Sending to a unit that already holds the problem replaces its order there; without --clean that is a send-back.""" @@ -307,7 +307,7 @@ def send(unit, due, body: str, clean=False, mechanical=False, reassign=None, new # A send-back is a strike, no warnings, and every send-back is roasted in #comms. # Done here so the commander cannot send work back and forget either. verdict = next(l for l in body.splitlines() if BluntText.found(l)).strip() - strike(unit, group[0], f'sent back on {group[0]}: {verdict}') + strike(unit, group[0], f'sent back on {group[0]}: {verdict}', evidence=evidence) def importers(modules: set[str], check: OrderCheck) -> list[str]: @@ -400,7 +400,7 @@ def board(): owner = unit or f'unassigned: released from {last[0].previous.owner}: {last[0].reason.text}' lines.append(f'| {t.id}: {t.text} | {t.order.owner.to_wire()} | {owner} | {t.order.id} | ' f'{hhmm(t.deadline)} | {read[t.id].line if t.id in read else "-"} | ' - f'{"no" if t.pending else "yes, install pending"} | ' + f'{t.order.progress_line} | ' f'{activity[unit].clock() if unit else "-"} |') history = COMMS.goals.todos.history_for_commander(COMMS.registry.require(ME).incarnation) lines += ['', '## Withdrawn orders', '', @@ -408,9 +408,6 @@ def board(): open(BOARD, 'w').write('\n'.join(lines) + '\n') -STAGES = ('tool call', 'review', 'install', 'live', 'user') # where a mistake was caught; later is worse - - def on_record(line: str, recorded: str, audience: VerdictAudience) -> None: """Put one line on the public record: the narrative file, then its audience. @@ -431,24 +428,20 @@ def on_record(line: str, recorded: str, audience: VerdictAudience) -> None: def strike(unit: str, task: str, why: str, caught: str = f'{ME} at review', *, - kind: type[StrikeTodo] = StrikeTodo) -> None: + evidence: tuple[Evidence, ...] = (), kind: type[StrikeTodo] = StrikeTodo) -> None: """HARD RULE: every failure goes on the public record, in plain words, damning and exact, so no failure stays private and none is described in words that hide what happened.""" - # What matters is how a mistake happened, how often, and how and when it was caught. - if not any(stage in caught for stage in STAGES): - sys.exit(f'Refused: --caught must say who caught it and at which stage {STAGES}, e.g. "{ME} at review".') todo = COMMS.goals.todos.get(task) todo.state.require_open(todo) if todo.held().owner != unit: sys.exit(f'Refused: {unit} does not hold {task}; strike the unit that holds it.') recorded(unit, todo, kind(todo.revision, COMMS.registry.require(ME).incarnation, todo.held(), - f'{why} (caught by {caught})')) + f'{why} (caught by {caught})', evidence=evidence)) def recorded(unit: str, todo: Todo, strike: StrikeTodo) -> None: """Core records the strike on the task's Todo; it is written to the narrative file and posted to #comms in the same step.""" - refuse(strike.reason, *OUTGOING) # posted to #comms, so refused before Core records the strike if call(CommsTodoTool(id=todo.id, operation=strike)).todo.revision == todo.revision: return posted(unit, COMMS.goals.todos.get(todo.id), strike) @@ -500,17 +493,15 @@ def done(pid, sha, screenshot='', seen='', repo=''): claim = WorkingClaim(evidence.artifact()) else: delivered_commit = revision(repo, sha) - if subprocess.run(['git', '-C', repo, 'merge-base', '--is-ancestor', - delivered_commit, 'HEAD']).returncode: - sys.exit(f'Refused: {delivered_commit} is not in {repo} HEAD.') delivery = CommitDelivery(commit=revision(repo, 'HEAD'), delivered_commit=delivered_commit, evidence=evidence) - claim = MergedClaim(delivery.delivered) - call(CommsOrderDoneTool(problem=pid, order_id=todo.order.id, delivery=delivery, claims=(claim,))) + claims = (claim,) if operation else () + call(CommsOrderDoneTool(problem=pid, order_id=todo.order.id, delivery=delivery, claims=claims)) delivered_todo = COMMS.goals.todos.get(pid) for late in delivered_todo.strikes[len(todo.strikes):]: posted(unit, delivered_todo, late) - print(f'{unit}: order {todo.order.id} delivered as {delivery.delivered if not operation else sha}') + print(f'{unit}: order {todo.order.id} delivered as {delivery.delivered if not operation else sha}' + f' · {delivered_todo.order.delivery.check_line}') def first_calls(held, activity): @@ -763,8 +754,26 @@ class OrderVerb(CommanderVerb): """A verb of `order`.""" +class CitedState: + """Derive command options from the Evidence declarations, not another kind roster.""" + + def __init_subclass__(cls) -> None: + cls.__annotations__ = {**cls.__annotations__, + **{member.form.split()[0].removeprefix('--'): tuple[str, ...] + for member in Evidence.members_with(Evidence)}} + for member in Evidence.members_with(Evidence): + flag, metavar = member.form.split() + setattr(cls, flag.removeprefix('--'), option( + flag, action='append', default=(), parser_default_factory=list, metavar=metavar)) + super().__init_subclass__() + + def evidence(self) -> tuple[Evidence, ...]: + return tuple(member.cited(value) for member in Evidence.members_with(Evidence) + for value in getattr(self, member.form.split()[0].removeprefix('--'))) + + @dataclass(frozen=True, kw_only=True) -class SendVerb(OrderVerb, RuledVerb): +class SendVerb(CitedState, OrderVerb, RuledVerb): help = "record the order in Core and send it" unit: str = option("unit") due: str = option("due", help="HH:MM local time") @@ -780,21 +789,19 @@ class SendVerb(OrderVerb, RuledVerb): def run(self) -> None: send(self.unit, self.due, Path(self.body_file).read_text(), clean=self.clean, mechanical=self.mechanical, reassign=self.reassign, traced=self.traced, - replaces=self.replaces, disk=self.disk) + replaces=self.replaces, disk=self.disk, evidence=self.evidence()) @dataclass(frozen=True, kw_only=True) -class StrikeVerb(OrderVerb, RuledVerb): +class StrikeVerb(CitedState, OrderVerb, RuledVerb): help = "a send-back, a missed deadline or a false claim" unit: str = option("unit") why: str = option("--why") - caught: str = option("--caught", default=f'{ME} at review', help='"WHO at STAGE"; stage is review/install/live/user') + pid: str = option("pid", help="the problem the strike is for") + caught: str = option("--caught", default=f'{ME} at review') def run(self) -> None: - references = set(re.findall(r'\bP\d+\b', self.why)) - if len(references) != 1: - sys.exit('Refused: name exactly one problem in --why; strikes count per problem.') - strike(self.unit, references.pop(), self.why, self.caught) + strike(self.unit, self.pid, self.why, self.caught, evidence=self.evidence()) @dataclass(frozen=True, kw_only=True) diff --git a/src/agent_comms/commander/standing.py b/src/agent_comms/commander/standing.py index 4232f88329..e845b3ee2a 100644 --- a/src/agent_comms/commander/standing.py +++ b/src/agent_comms/commander/standing.py @@ -15,7 +15,7 @@ UndatedStandingRule) from ..goals import Goal from ..threads import Thread -from ..todos import FlatTrend, OrderProgress, PullRequests, StrikeTodo, Todo +from ..todos import FlatTrend, OrderProgress, PullRequests, StrikeTodo, Todo, RuntimeEvidence from ..turn_digests import TurnDigestRead, TurnsSince from . import judge from .config import CommanderConfig, Repo @@ -124,7 +124,7 @@ def enforce(self, comms: Comms, commander: str) -> None: comms.goals.change_problem(todo.id, StrikeTodo( todo.revision, comms.registry.require(commander).incarnation, held, f"{held.owner} kept working on {todo.id} while the distance to done did not fall" - " across two readings"), commander) + " across two readings", evidence=(RuntimeEvidence(str(comms.root)),)), commander) for unit in self.idle(comms, commander)[:1]: self.assign(comms, commander, unit) diff --git a/src/agent_comms/connector.py b/src/agent_comms/connector.py index 9ff7e81d2a..f4f4230bcd 100644 --- a/src/agent_comms/connector.py +++ b/src/agent_comms/connector.py @@ -37,6 +37,7 @@ from aiohttp import web from pydantic import JsonValue +from .channel_targets import CommandTarget from .cli_commands import DeclaredOptions, option from .cli_protocol import CoreError from .comms import Comms, client_wire @@ -49,7 +50,7 @@ RegisteredClient, digest, ) from .messages import Message -from .todos import DirectiveText +from .todos import AuthorizedAct, DirectiveText from .tool_caller import ThreadCaller from .tools import CommsChannelsTool, ReadTool, ToolRequest @@ -62,9 +63,11 @@ class DirectorCaller(ThreadCaller): """claude-ai speaks for the owner: each message it sends a thread is a directive, recorded as a request when that thread holds owner requests.""" - def require_directive(self, comms: Comms, body: str) -> None: - """A directive passes DirectiveText. claude-ai directs the commanders, so the - commander rules review does not gate it.""" + def require_directive(self, comms: Comms, target: str, body: str) -> None: + """Refuse routine permission questions to humans; the owner needs no commander review.""" + for name in CommandTarget.named(target).direct_names: + for human_body in comms.registry.require(name).role.human(body): + AuthorizedAct.require(human_body) DirectiveText.require(body) def sent(self, comms: Comms, message: Message) -> None: diff --git a/src/agent_comms/goals.py b/src/agent_comms/goals.py index 85653a9dcc..5918180c7b 100644 --- a/src/agent_comms/goals.py +++ b/src/agent_comms/goals.py @@ -539,7 +539,7 @@ def goal_text(self, goal: Goal) -> str: def progress(self, goal: Goal, todos: Sequence[Todo]) -> str: """Each problem's order state and latest verdict, as Core records them on its Todo.""" - return "\n".join(" · ".join((f"{todo.id}: {todo.order.line}", + return "\n".join(" · ".join((f"{todo.id}: {todo.order.progress_line}", *(verdict.line for verdict in todo.verdicts[-1:]))) for todo in todos) diff --git a/src/agent_comms/todos.py b/src/agent_comms/todos.py index 6a9d18f50a..f02dd214f9 100644 --- a/src/agent_comms/todos.py +++ b/src/agent_comms/todos.py @@ -25,7 +25,7 @@ from . import skills from .change_report import FileLine -from .child_process import BoundedRun, CheckPrinted, ChildResult, ExitedOutcome +from .child_process import BoundedRun, CheckPrinted, ChildResult, ExitedOutcome, Platform, ProcessIdentity from .command import Command from .declared_family import DeclaredFamily from .errors import ToolRefusedError @@ -38,6 +38,7 @@ from .recorded_time import EventTime, RecordedEventTime, UnrecordedEventTime from .thread_identity import RecordedThreadIncarnation, ThreadIncarnation from .threads import Thread +from .turn_digests import TurnDigestRead, TurnsSince from .typed_table import Column, Index, JsonStorage, TypedStore, TypedTable from .standing_rules import UndatedStandingRule from .storage_rules import StorageRule @@ -85,12 +86,20 @@ async def run(self, check, *command): raise ValueError(result.stderr.decode(errors="replace") or "Delivery source check failed.") return result.stdout.decode(errors="replace") - async def verified(self, check: OrderCheck, deletes: OrderDeletion) -> OrderDelivery: - """The order's check and every count its deletion declares run on the delivered source.""" - for each in (check, *deletes.checks): + @property + def check_line(self) -> str: + return "Check print not recorded" + + async def verified(self, check: OrderCheck, deletes: OrderDeletion) -> CheckedDelivery: + """Record the order's actual check output, not an integer supplied by its reporter.""" + await self.require_source(check) + result = await self.checked(check) + check.result.require(result) + for each in deletes.checks: await self.require_source(each) each.result.require(await self.checked(each)) - return replace(self, at=await self.delivery_time(check)) + delivery = replace(self, at=await self.delivery_time(check)) + return CheckedDelivery.record(delivery, CheckPrinted.decoded(result.stdout)) @abstractmethod async def require_source(self, check): ... @@ -145,7 +154,8 @@ def strikes( lateness = self.at.lateness(todo.deadline) reason = (f"{todo.id} order {order_id} {lateness} as {self.delivered} " f"(caught by {actor.name} at review)") - return (StrikeTodo(todo.revision, actor, todo.held(), reason),) * lateness.late() + return (StrikeTodo(todo.revision, actor, todo.held(), reason, + evidence=(CommitEvidence(self.delivered),)),) * lateness.late() @dataclass(frozen=True, kw_only=True) @@ -187,6 +197,35 @@ async def delivery_time(self, check): return EventTime.capture(time.time()) +@dataclass(frozen=True, kw_only=True) +class CheckedDelivery(OrderDelivery, declared_name=None): + """A checked delivery keeps its actual measured print.""" + + printed: CheckPrinted + + @property + def check_line(self) -> str: + return self.printed.line + + @classmethod + def record(cls, delivery: OrderDelivery, printed: CheckPrinted) -> CheckedDelivery: + for member in cls.members_with(cls): + if issubclass(member, type(delivery)): + values = {item.name: getattr(delivery, item.name) for item in fields(delivery)} + return member(**(values | {"printed": printed})) + raise ValueError(f"No checked receipt declared for {type(delivery).__name__}.") + + +@dataclass(frozen=True, kw_only=True) +class CheckedCommitDelivery(CheckedDelivery, CommitDelivery): + pass + + +@dataclass(frozen=True, kw_only=True) +class CheckedOperationDelivery(CheckedDelivery, OperationDelivery): + pass + + @dataclass(frozen=True) class PullRequest: number: int @@ -971,6 +1010,107 @@ def render(self): f'{self.by.to_wire()} owns it; {self.derive.sentence}, so the copy is deleted.') +@dataclass(frozen=True) +class PermissionQuestion: + """An interrogative clause, tokenized once before asking the act declarations.""" + + words: tuple[str, ...] + modals: ClassVar[frozenset[str]] = frozenset({"should", "shall", "may", "can", "could"}) + subjects: ClassVar[frozenset[str]] = frozenset({"i", "we"}) + + @classmethod + def read(cls, text: str) -> Iterator[PermissionQuestion]: + for question in OrderWords.CITED.sub(" ", text).split("?")[:-1]: + clause = re.split(r"[.!;\n]", question)[-1] + yield cls(tuple(re.findall(r"[a-z]+", clause.casefold()))) + + def requests(self, verbs: tuple[str, ...]) -> bool: + for at, word in enumerate(self.words): + if word not in verbs: + continue + subject = self.words[:at] + if subject[-2:] == ("me", "to"): + return True + if len(subject) == 2 and subject[0] in self.modals and subject[1] in self.subjects: + return True + return False + + +class AuthorizedAct(DeclaredFamily, affix="Act"): + """Routine acts the director performs without asking the human to authorize them again. + + Product questions and acts outside this family remain questions for the owner. The + shared question grammar is not a roster of permission phrases or action-name dispatch. + """ + + other_verbs: ClassVar[tuple[str, ...]] = () + + @classmethod + def asked(cls, question: PermissionQuestion) -> bool: + return question.requests((cls.declared_name, *cls.other_verbs)) + + @classmethod + def require(cls, text: str) -> None: + for question in PermissionQuestion.read(text): + for act in cls.members_with(cls): + if act.asked(question): + raise ToolRefusedError( + f"Already authorized: {act.declared_name}. Do it and report the result; " + "do not ask the user to authorize this routine act again.") + + +class SendAct(AuthorizedAct): + other_verbs = ("post", "forward", "relay") + + +class EnforceAct(AuthorizedAct): + pass + + +class StrikeAct(AuthorizedAct): + pass + + +class ReassignAct(AuthorizedAct): + other_verbs = ("move",) + + +class FixAct(AuthorizedAct): + other_verbs = ("repair", "patch") + + +class RegisterAct(AuthorizedAct): + pass + + +class RestartAct(AuthorizedAct): + pass + + +class InstallAct(AuthorizedAct): + pass + + +class MergeAct(AuthorizedAct): + pass + + +class KillAct(AuthorizedAct): + other_verbs = ("stop",) + + +class ArchiveAct(AuthorizedAct): + pass + + +class RetryAct(AuthorizedAct): + other_verbs = ("clear",) + + +class StartAct(AuthorizedAct): + pass + + class DirectiveText: """What the commander and the owner's voice send units: order send, order say and the connector's send refuse a text failing any of these.""" @@ -1114,6 +1254,13 @@ class ProblemOrder(DeclaredFamily, affix="ProblemOrder"): finding: ProblemFinding = field(default_factory=UnlocatedProblemFinding, metadata={"wire_omit_default": True}) + @property + def progress_line(self) -> str: + return self.line + + def verify_evidence(self, evidence: tuple[Evidence, ...], goals: Goals) -> None: + raise ValueError("This problem has no checked order to cite evidence against.") + @classmethod def pull_request_ids(cls, body: str) -> tuple[str, ...]: return tuple(line.removeprefix(cls.pull_request_label).strip() @@ -1196,6 +1343,10 @@ class CheckedProblemOrder(ProblemOrder, declared_name=None): default_factory=NoOrderDeletion, metadata={"wire_omit_default": True}) serves: type[OrderPhase] = field(default=UnplannedPhase, metadata={"wire_omit_default": True}) + def verify_evidence(self, evidence: tuple[Evidence, ...], goals: Goals) -> None: + for cited in evidence: + cited.verify(goals, self) + def __post_init__(self): if not self.edits or not self.banned: raise ValueError("An order requires edits and banned actions.") @@ -1342,6 +1493,10 @@ class DeliveredProblemOrder(CheckedProblemOrder): pending: ClassVar[bool] = False line: ClassVar[str] = "Delivered, waiting on install" + @property + def progress_line(self) -> str: + return f"{self.line} · {self.delivery.check_line}" + def require_transfer(self, todo: Todo) -> None: owner = (todo.assignment or todo.released[-1].previous).owner raise ValueError(f"{owner} already has {self.delivery.work}; {owner} merges it.") @@ -1661,19 +1816,125 @@ def apply(self, current: Todo) -> Todo: revision=current.revision + 1) +@dataclass(frozen=True) +class Evidence(DeclaredFamily, affix="Evidence"): + """Checkable state behind a strike; a board row is not an artifact.""" + form: ClassVar[str] + + @classmethod + @abstractmethod + def cited(cls, text: str) -> Self: ... + + @classmethod + def verified(cls, evidence: tuple[Evidence, ...], goals: Goals, current: Todo) -> None: + if not evidence: + raise ValueError("Cite the state the strike stands on: " + + ", ".join(member.form for member in cls.members_with(cls)) + + "; a board row, goal text or another agent's account is not state.") + current.order.verify_evidence(evidence, goals) + + @abstractmethod + def verify(self, goals: Goals, order: CheckedProblemOrder) -> None: ... + + @staticmethod + def git(order: CheckedProblemOrder, *args: str) -> int: + return subprocess.run(("git", "-C", order.check.directory, *args), + capture_output=True, timeout=30).returncode + + +@dataclass(frozen=True) +class CommitEvidence(Evidence): + sha: str + form = "--commit SHA" + + @classmethod + def cited(cls, text: str) -> Self: + return cls(text) + + def verify(self, goals: Goals, order: CheckedProblemOrder) -> None: + if self.git(order, "cat-file", "-e", f"{self.sha}^{{commit}}"): + raise ValueError(f"{self.sha} is no commit of {order.check.directory}") + + +@dataclass(frozen=True) +class DiffEvidence(Evidence): + a: str + b: str + form = "--diff A..B" + + @classmethod + def cited(cls, text: str) -> Self: + return cls(*text.split("..")) + + def verify(self, goals: Goals, order: CheckedProblemOrder) -> None: + if self.git(order, "diff", "--quiet", self.a, self.b) != 1: + raise ValueError(f"{self.a}..{self.b} changes nothing in {order.check.directory}") + + +@dataclass(frozen=True) +class TurnEvidence(Evidence): + thread: str + turn_id: str + form = "--turn THREAD:TURN_ID" + + @classmethod + def cited(cls, text: str) -> Self: + return cls(*text.split(":", 1)) + + def verify(self, goals: Goals, order: CheckedProblemOrder) -> None: + turns = TurnDigestRead(goals.root, goals.registry.require(self.thread), + TurnsSince(order.at.recorded_at("An order"))).read().turns + if self.turn_id not in {turn.turn_id for turn in turns}: + raise ValueError(f"{self.thread} recorded no turn {self.turn_id} since this order") + + +@dataclass(frozen=True) +class ProcessEvidence(Evidence): + pid: int + start: int + form = "--process PID:START" + + @classmethod + def cited(cls, text: str) -> Self: + pid, start = text.split(":") + return cls(int(pid), int(start)) + + def verify(self, goals: Goals, order: CheckedProblemOrder) -> None: + if not Platform.current().matches(ProcessIdentity(self.pid, self.start)): + raise ValueError(f"No process {self.pid} started at {self.start} is running") + + +@dataclass(frozen=True) +class RuntimeEvidence(Evidence): + path: str + form = "--runtime PATH" + + @classmethod + def cited(cls, text: str) -> Self: + return cls(text) + + def verify(self, goals: Goals, order: CheckedProblemOrder) -> None: + if not Path(self.path).is_absolute() or not Path(self.path).exists(): + raise ValueError(f"{self.path} is no existing absolute runtime path") + + @dataclass(frozen=True) class StrikeTodo(TodoOperation): actor: ThreadIncarnation previous: Assignment reason: str at: EventTime = field(default_factory=lambda: EventTime.capture(time.time())) + evidence: tuple[Evidence, ...] = field(default=(), metadata={"wire_omit_default": True}) + + def prepare(self, goals: Goals, current: Todo) -> Self: + Evidence.verified(self.evidence, goals, current) + return self def apply(self, current: Todo) -> Todo: current.require_revision(self.expected_revision) current.require_commander(self.actor) current.require_assignment(self.previous) _text(self.reason, "Strike reason", 2000) - StrikeReason(self.reason) if any(self.repeats(recorded) for recorded in current.live_strikes(self.previous.owner)): return current return replace(current, strikes=(*current.strikes, self), revision=current.revision + 1) @@ -1800,15 +2061,20 @@ def judged(self, current: Todo) -> Todo: @dataclass(frozen=True) class SendBackVerdict(OrderVerdict): """The tool call goes back; the same write strikes the holder and revises the order Goal.""" - reason: StrikeReason + reason: str + evidence: tuple[Evidence, ...] = field(default=(), metadata={"wire_omit_default": True}) + + def prepare(self, goals: Goals, current: Todo) -> Self: + Evidence.verified(self.evidence, goals, current) + return self @property def line(self) -> str: - return f"Sent back: {self.reason.text}" + return f"Sent back: {self.reason}" def judged(self, current: Todo) -> Todo: - strike = StrikeTodo(self.expected_revision, self.actor, self.previous, self.reason.text, - self.at) + strike = StrikeTodo(self.expected_revision, self.actor, self.previous, self.reason, + self.at, evidence=self.evidence) return strike.apply(current) def record(self, goals: Goals, problem: str) -> Todo: diff --git a/src/agent_comms/tool_caller.py b/src/agent_comms/tool_caller.py index 00723d54df..9825395ecb 100644 --- a/src/agent_comms/tool_caller.py +++ b/src/agent_comms/tool_caller.py @@ -45,8 +45,8 @@ def subject(self, named: str) -> str: ... def may_reconfigure(self, comms: Comms, name: str) -> bool: ... @abstractmethod - def require_directive(self, comms: Comms, body: str) -> None: - """A caller whose sends direct units refuses a body its directive checks refuse.""" + def require_directive(self, comms: Comms, target: str, body: str) -> None: + """The calling role checks its directive for this destination before publication.""" @abstractmethod def sent(self, comms: Comms, message: Message) -> None: @@ -75,7 +75,7 @@ def subject(self, named: str) -> str: def may_reconfigure(self, comms: Comms, name: str) -> bool: return not comms.registry.status(name).active - def require_directive(self, comms: Comms, body: str) -> None: + def require_directive(self, comms: Comms, target: str, body: str) -> None: """A caller with no thread gives no directive.""" def sent(self, comms: Comms, message: Message) -> None: @@ -103,7 +103,7 @@ def may_reconfigure(self, comms: Comms, name: str) -> bool: return (not comms.registry.status(name).active or comms.registry.canonical_name(name) == comms.registry.canonical_name(self.name)) - def require_directive(self, comms: Comms, body: str) -> None: + def require_directive(self, comms: Comms, target: str, body: str) -> None: """A unit's or a script's send is a message, not a directive.""" def sent(self, comms: Comms, message: Message) -> None: diff --git a/src/agent_comms/tools.py b/src/agent_comms/tools.py index f07fe9a1eb..5b2c7bd61a 100644 --- a/src/agent_comms/tools.py +++ b/src/agent_comms/tools.py @@ -871,11 +871,9 @@ class CommsOrderDoneTool(ThreadToolRequest): order_id: str = tool_field("Exact open order ID") delivery: OrderDelivery = tool_field("A clean commit delivery or an operation verified by the order's check") claims: tuple[ProgressClaim, ...] = tool_field( - "Merged, installed or working claims, each with the artifact Core verifies in the order's " - "checkout; at least one") + "Additional application claims; the delivery owns commit verification", default=()) def apply(self, comms: Comms, caller: ThreadCaller) -> TodoChanged: - ProgressClaim.require_claimed(self.claims, "delivered") todo = comms.goals.todos.get(self.problem) ProgressClaim.require_all(self.claims, comms, Path(todo.order.check.directory)) todo.require_commander(comms.registry.require(caller.name).incarnation) @@ -1079,7 +1077,7 @@ class CommsSendTool(ToolRequest): def apply(self, comms: Comms, caller: ToolCaller) -> MessagePublished: try: - caller.require_directive(comms, self.body) + caller.require_directive(comms, self.target, self.body) body = VerdictAudience(self.audience).addressed(self.target, self.body) result = comms.messaging.send(self.sender, self.target, body, self.type, report=self.report) except ToolRefusedError as error: diff --git a/tests/test_commander_problem_orders.py b/tests/test_commander_problem_orders.py index 2d7c880cdc..b96b80d3bb 100644 --- a/tests/test_commander_problem_orders.py +++ b/tests/test_commander_problem_orders.py @@ -167,6 +167,26 @@ def test_delivery_checks_once_and_preserves_the_units_goal(commander): assert comms.registry.require('worker').goal == original_goal +def test_core_done_records_the_check_print_even_when_a_caller_supplies_a_false_receipt(commander): + from agent_comms.child_process import CountPrinted + from agent_comms.todos import CheckedOperationDelivery + + comms, caller = commander + marker = comms.root / 'measured-check-calls' + for number, supplied in enumerate((OperationDelivery(), + CheckedOperationDelivery(printed=CountPrinted(999))), 1): + pid = f'P{number}' + todo = create(commander, pid, f"printf x >> '{marker}'; printf 7", 7) + CommsOrderDoneTool(problem=pid, order_id=todo.order.id, + delivery=supplied).apply(comms, caller) + recorded = TodoStore(comms.root).get(pid) + assert recorded.order.delivery.printed == CountPrinted(7) + assert recorded.order.delivery.check_line == 'Check printed 7' + assert 'Check printed 7' in recorded.order.progress_line + assert FieldCodec.decode(Todo, FieldCodec.encode(recorded)) == recorded + assert marker.read_text() == 'xx' # Exactly one actual check per delivery. + + def test_failed_check_keeps_problem_pending(commander): comms, caller = commander original = create(commander, 'P1', 'printf 9', 1) diff --git a/tests/test_connector.py b/tests/test_connector.py index 15c389300a..1c270c8555 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -261,6 +261,42 @@ async def test_connector_thread_tools_act_as_claude_ai(client, store, comms): ("claude-ai", "Owner said go")] +async def test_director_checks_human_permission_questions(client, store, comms): + from agent_comms.coordination_tables.refusals import SendRefusal + from agent_comms.thread_identity import UserThreadRole + + comms.registry.declare(comms.registry.new_thread( + 'human', frozenset(), str(comms.root), role=UserThreadRole, created_at=3.0)) + reviews = RuleReviews(comms.root) + reviews.write('claude-ai', DrawnRuleReviewEntry(('Declare the act once',), 1)) + reviews.write('claude-ai', AnsweredRuleReviewEntry( + 1, 'P192 uses the existing caller role and the act family before anything is sent.')) + headers = {'Authorization': f'Bearer {await approved_token(client, store)}'} + + async def send(target, body): + response = await client.post('/mcp', headers=headers, json={ + 'jsonrpc': '2.0', 'id': 1, 'method': 'tools/call', + 'params': {'name': 'comms_send', 'arguments': { + 'from': 'claude-ai', 'to': target, 'body': body}}}) + assert response.status == 200 + return (await response.json())['result'] + + before = comms.bus.log.latest_sequence() + for question in ('Should I register the worker?', 'Want me to send the order?', + 'Would you like me to restart the connector?'): + refusal = await send('human', question) + assert refusal['isError'] and 'Already authorized' in str(refusal) + assert comms.bus.log.latest_sequence() == before + assert len(SendRefusal.recent(comms.root, sender='claude-ai', target='human', limit=10)) == 3 + for question in ('Should the next screen show tasks or projects?', + 'Should I buy a license for a different paid provider?', + 'Did I restart the connector?', + 'The log says `Should I restart the connector?`.'): + assert not (await send('human', question))['isError'] + assert not (await send('worker', 'Want me to send the order?'))['isError'] + assert comms.bus.log.latest_sequence() == before + 5 + + async def test_a_call_returns_what_the_tool_returns_in_process(client, store, root): token = await approved_token(client, store) called = await (