diff --git a/plans/commander-native-plan.md b/plans/commander-native-plan.md index dbbe1eac22..88aafe180e 100644 --- a/plans/commander-native-plan.md +++ b/plans/commander-native-plan.md @@ -38,7 +38,7 @@ | 5 | Board, overdue, silent, spinning | `order.py board`, `overdue`; `fleet-board.md` | `CommsTodoListTool` and the fleet tool, derived | `one_order_owner` | | 6 | Branch hygiene | `branches-*.txt` by hand | `CommsWorkBranchesTool` | `one_order_owner` | | 7 | Strikes and the strike limit | `order.py strike`; `strikes.jsonl` | `Strike` rows, author-declared `StrikeLimit`, one `comms_fire` | `record_plane` | -| 8 | Verdicts on a unit's tool calls | `judge.py ok`, `back`; `verdicts.jsonl` | Order `Verdict`; send-back revises the Goal and records its strike atomically | `record_plane` | +| 8 | Verdicts on a unit's tool calls | `judge.py ok`, `back`; both invoke `CommsVerdictTool` | Order `Verdict`; send-back revises the Goal and records its strike atomically | `record_plane` | | 9 | Plain words in verdicts and strikes | judge and order checks | `PlainText` | `record_plane` | | 10 | Refuse soft or vague orders | `order.py` | `OrderText` on every order action | `record_plane` | | 11 | What a unit did during its order | `tool_watch.py`; `edits.jsonl`; `judge.py show` | `comms_order_work`: both transcript families by `tool_call_id` | `review_queue` | diff --git a/src/agent_comms/commander_parity/deletion.py b/src/agent_comms/commander_parity/deletion.py index dd161af5a4..b49e85802f 100644 --- a/src/agent_comms/commander_parity/deletion.py +++ b/src/agent_comms/commander_parity/deletion.py @@ -27,6 +27,7 @@ CheckedCommitDelivery, CommitDelivery, DeliveredProblemOrder, + FailedCheckStrike, FamilyDeletion, MechanicalOrderRun, Todo, @@ -138,6 +139,10 @@ def exercise(cls, run: LifecycleRun) -> LifecycleObservation: output = io.StringIO() with contextlib.redirect_stdout(output): before = cls.send(run, document) + goal_threads = (run.commander.commander, run.worker) + before_goals = tuple( + run.commander.comms.registry.require(name).goal for name in goal_threads + ) partial = cls.commit( repository, family + "def decisions():\n return []\n", @@ -148,6 +153,17 @@ def exercise(cls, run: LifecycleRun) -> LifecycleObservation: ) after_refusal = run.commander.comms.goals.todos.get(before.id) remaining = count.recorded(partial, str(repository)) + after_refusal_goals = tuple( + run.commander.comms.registry.require(name).goal for name in goal_threads + ) + failed_check = False + match after_refusal.strikes[len(before.strikes) :]: + case (FailedCheckStrike() as strike,): + failed_check = strike == FailedCheckStrike.make( + before, strike.at.recorded_at("The failed delivery"), strike.reason + ) and after_refusal == replace( + before, strikes=(*before.strikes, strike), revision=before.revision + 1 + ) complete = cls.commit( repository, "def decisions():\n return []\n", @@ -171,7 +187,9 @@ def exercise(cls, run: LifecycleRun) -> LifecycleObservation: bool(remaining) and count.result.distance(remaining[0]) == CountPrinted(1), ), ("nonzero delivery refused", refused.outcome == ExitedOutcome(1)), - ("refusal preserved pending order", before == after_refusal and after_refusal.pending), + ("refusal kept the order pending", after_refusal.pending), + ("refusal preserved thread goals", after_refusal_goals == before_goals), + ("refusal recorded only its failed-check strike", failed_check), ( "zero count checked on delivered commit", bool(zero) and count.result.distance(zero[0]) == CountPrinted(0), diff --git a/src/agent_comms/commander_parity/rows.py b/src/agent_comms/commander_parity/rows.py index 9378cb8541..f49d1cecc2 100644 --- a/src/agent_comms/commander_parity/rows.py +++ b/src/agent_comms/commander_parity/rows.py @@ -23,6 +23,7 @@ from .load import LoadComparison from .measurements import StorageComparison, measured_load from .story import StoryCase +from agent_comms.commander_parity.verdicts import VerdictComparison class CreateProblemParityRow(ParityRow): @@ -93,9 +94,10 @@ class StrikeLimitParityRow(ParityRow): class VerdictParityRow(ParityRow): number = 8 capability = "Verdicts on a unit's tool calls" - current = "`judge.py ok`, `back`; `verdicts.jsonl`" + current = "`judge.py ok`, `back`; both invoke `CommsVerdictTool`" native = "Order `Verdict`; send-back revises the Goal and records its strike atomically" phases = (RecordPlanePhase,) + comparison = VerdictComparison.read class PlainWordsParityRow(ParityRow): diff --git a/src/agent_comms/commander_parity/verdict_edit.mjs b/src/agent_comms/commander_parity/verdict_edit.mjs new file mode 100644 index 0000000000..8c5b5bc257 --- /dev/null +++ b/src/agent_comms/commander_parity/verdict_edit.mjs @@ -0,0 +1,23 @@ +// A controlled request to the installed native edit tool; persist its actual result. +import { pathToFileURL } from 'node:url'; +import { readFile } from 'node:fs/promises'; +const [packagePath, sessionPath, path, callId, replacement] = process.argv.slice(2); +const sdk = pathToFileURL(`${packagePath}/dist/index.js`).href; +const { createEditTool, SessionManager } = await import(sdk); +const args = {path, edits: [{oldText: await readFile(path, 'utf8'), newText: replacement}]}; +const session = SessionManager.open(sessionPath); +session.appendMessage({role: 'user', content: 'Execute one isolated verdict-check edit.', + timestamp: Date.now()}); +session.appendMessage({ + role: 'assistant', content: [{type: 'toolCall', id: callId, name: 'edit', arguments: args}], + api: 'controlled-verification', provider: 'controlled-verification', model: 'local-native-edit', + usage: {input: 0, output: 0, cacheRead: 0, cacheWrite: 0, totalTokens: 0, + cost: {input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0}}, + stopReason: 'toolUse', timestamp: Date.now(), +}); +const result = await createEditTool(process.cwd()).execute(callId, args); +if ((await readFile(path, 'utf8')) !== replacement) { + throw new Error('Native edit did not apply its request'); +} +session.appendMessage({role: 'toolResult', toolCallId: callId, toolName: 'edit', + content: result.content, details: result.details, isError: false, timestamp: Date.now()}); diff --git a/src/agent_comms/commander_parity/verdicts.py b/src/agent_comms/commander_parity/verdicts.py new file mode 100644 index 0000000000..b867b26a87 --- /dev/null +++ b/src/agent_comms/commander_parity/verdicts.py @@ -0,0 +1,270 @@ +"""Judge/native verdicts on actual native edits in LifecycleRun's disposable bus.""" + +from __future__ import annotations + +import contextlib +import io +import json +import os +from abc import ABC, abstractmethod +from collections.abc import Callable +from dataclasses import dataclass +from pathlib import Path +from typing import ClassVar +from uuid import uuid4 + +from agent_comms.active_route import resolve_comms_route +from agent_comms.commander.judge import BackVerb, Edit, OkVerb, VerdictVerb +from agent_comms.commander.rules import AnswerVerb, ReviewVerb +from agent_comms.rule_reviews import DrawnRule, RuleReviews +from agent_comms.todos import AcceptedVerdict, SendBackVerdict, Todo, ToolCallVerdict +from agent_comms.tools import CommsVerdictTool, TodoChanged +from agent_comms.turn_context import GoalSegment +from agent_comms.commander_parity import ParityComparison +from agent_comms.commander_parity.lifecycle import LifecycleObservation, LifecycleRun + +NATIVE_EDIT = Path(__file__).with_name("verdict_edit.mjs") + + +@dataclass(frozen=True) +class VerdictMutation: + call_id: str + verdicts: int + todo_revisions: int + strikes: int + goal_revisions: int + goal_history_entries: int + progress: str + + def checks(self, changes: int) -> tuple[tuple[str, bool], ...]: + return ( + ("one verdict", self.verdicts == 1), + ("one Todo revision", self.todo_revisions == 1), + ("strike delta", self.strikes == changes), + ("Goal revision delta", self.goal_revisions == changes), + ("committed Goal history delta", self.goal_history_entries == changes), + ) + + +@dataclass(frozen=True) +class VerdictObservation(LifecycleObservation): + mutation: VerdictMutation + + +class VerdictCase(ABC): + changes: ClassVar[int] + native_owner: ClassVar[type[ToolCallVerdict]] + + @classmethod + @abstractmethod + def replacement(cls, run: LifecycleRun) -> str: ... + + @classmethod + @abstractmethod + def command(cls, run: LifecycleRun, todo: Todo, edit: Edit) -> VerdictVerb: ... + + @staticmethod + def judge(run: LifecycleRun, command: VerdictVerb, todo: Todo, edit: Edit) -> None: + command.apply(run.commander) + + @staticmethod + def native(run: LifecycleRun, command: VerdictVerb, todo: Todo, edit: Edit) -> object: + return run.commander.call( + CommsVerdictTool( + problem=todo.id, operation=command.operation(run.commander, todo, edit.id) + ) + ) + + @classmethod + def capture( + cls, call_id: str, invoke: Callable[[LifecycleRun, VerdictVerb, Todo, Edit], object] + ) -> VerdictObservation: + with LifecycleRun.open() as run, contextlib.redirect_stdout(io.StringIO()): + # Satisfy the actual gate in this copy, without transferring original authority. + ReviewVerb().apply(run.commander) + with RuleReviews(run.commander.root).snapshot() as db: + drawn = DrawnRule.select(db, reviewer=run.commander.commander, order_by=("n",)) + for rule in drawn: + AnswerVerb( + n=rule.n, + text=( + f'"{rule.rule}" This isolated comparison keeps verdict mutation in ' + "its existing native owner, preserves original sessions, and checks " + "the actual stored Goal and strike rather than a second state store: " + f"{Path(__file__).resolve()}:1" + ), + ).apply(run.commander) + before = run.create() + worker = run.commander.comms.registry.require(run.worker) + goal = worker.goal.require() + history = len(run.commander.comms.goals.goal_history(run.worker, goal_id=goal.id)) + session_file = worker.session_file + if not session_file: + raise ValueError("The copied unit must have its own recorded native session") + for path in (Path(session_file), run.decisions, Path(worker.worktree)): + path.resolve().relative_to(run.commander.root.resolve()) + package = resolve_comms_route().launch().require_private().validate() + child = run.invoke_command( + ( + "node", + "--no-global-search-paths", + str(NATIVE_EDIT), + str(package.path), + session_file, + str(run.decisions), + call_id, + cls.replacement(run), + ), + cwd=worker.worktree, + env=package.node_environment(dict(os.environ)), + ) + if not child.outcome.successful: + raise RuntimeError( + f"Native edit {child.outcome.summary}: {child.stderr.decode('utf-8')}" + ) + edits = Edit.unjudged(run.commander.comms, run.commander.commander) + if len(edits) != 1 or edits[0].id != call_id: + raise ValueError("The copied unit must expose exactly its one real native edit") + edit = edits[0] + invoke(run, cls.command(run, before, edit), before, edit) + saved = run.commander.comms.goals.todos.get(before.id) + worker = run.commander.comms.registry.require(run.worker) + after = worker.goal.require() + native = tuple( + (item.request, item.result) + for item in run.commander.invocations + if isinstance(item.request, CommsVerdictTool) + ) + if len(native) != 1 or native[0][0].operation.tool_call_id != call_id: + raise ValueError("The native verdict must judge this one original copied tool call") + request, result = native[0] + # goal_history returns committed revisions; its internal tags stay there. + history_entries = run.commander.comms.goals.goal_history(run.worker, goal_id=goal.id)[ + history: + ] + progress = GoalSegment.capture(worker, after, run.commander.comms.goals).progress + mutation = VerdictMutation( + request.operation.tool_call_id, + len(saved.verdicts) - len(before.verdicts), + saved.revision - before.revision, + saved.strike_count - before.strike_count, + after.revision - goal.revision, + len(history_entries), + progress, + ) + checks = ( + *mutation.checks(cls.changes), + ("same Goal identity", after.id == goal.id), + ("same Goal state", after.state == goal.state), + ("native result retained", result == TodoChanged(saved, saved.strike_count)), + ("native verdict retained", request.operation in saved.verdicts), + ( + "strike is the verdict event", + all( + strike.at == request.operation.at + and strike.previous == request.operation.previous + for strike in saved.strikes[len(before.strikes) :] + ), + ), + ("declared verdict kind", isinstance(request.operation, cls.native_owner)), + ( + "Goal publication retained", + all(entry.before == goal and entry.after == after for entry in history_entries), + ), + ("native verdict visible in Goal", request.operation.line in progress), + ) + return VerdictObservation( + f"{cls.native_owner.__module__}.{cls.native_owner.__qualname__}", + len(native), + checks, + "", + mutation, + ) + + +class AcceptedCase(VerdictCase): + changes = 0 + native_owner = AcceptedVerdict + + @classmethod + def replacement(cls, run: LifecycleRun) -> str: + return "[]" + + @classmethod + def command(cls, run: LifecycleRun, todo: Todo, edit: Edit) -> OkVerb: + return OkVerb( + ids=(f"{edit.id}:{edit.code()}",), + why="The native edit removes the copied workflow decision.", + serves="Remove the copied workflow decision", + lines=(edit.hunks[0].new_text,), + ) + + +class SendBackCase(VerdictCase): + changes = 1 + native_owner = SendBackVerdict + why: ClassVar[str] = "The edit duplicates the copied workflow decision instead of removing it." + + @classmethod + def replacement(cls, run: LifecycleRun) -> str: + return json.dumps([run.worker, run.worker]) + + @classmethod + def command(cls, run: LifecycleRun, todo: Todo, edit: Edit) -> BackVerb: + return BackVerb( + ids=(f"{edit.id}:{edit.code()}",), + msg=todo.messages[-1].message_id, + lines=(edit.hunks[0].new_text,), + why=cls.why, + roast=( + "This is stupid because the edit keeps two copies of the same decision " + "instead of deleting it." + ), + fix="Remove the copied decision and let the existing check measure its absence.", + pattern="AGENT-2", + fact="the copied workflow decision collection", + at=f"{run.decisions}:1", + derive="delete-copy", + by=f"{Path(__file__).resolve()}:1", + ) + + +@dataclass(frozen=True) +class VerdictComparison(ParityComparison): + accepted_current: VerdictObservation + accepted_native: VerdictObservation + back_current: VerdictObservation + back_native: VerdictObservation + + @property + def passed(self) -> bool: + observations = ( + self.accepted_current, + self.accepted_native, + self.back_current, + self.back_native, + ) + return ( + all(observation.passed for observation in observations) + and self.accepted_current.mutation == self.accepted_native.mutation + and self.back_current.mutation == self.back_native.mutation + ) + + def describe(self) -> str: + return ( + "accepted current/native equal=" + f"{self.accepted_current.mutation == self.accepted_native.mutation}; " + "send-back current/native equal=" + f"{self.back_current.mutation == self.back_native.mutation}; " + f"accepted={self.accepted_native.describe()}; send-back={self.back_native.describe()}" + ) + + @classmethod + def read(cls) -> VerdictComparison: + call_id = uuid4().hex + return cls( + AcceptedCase.capture(call_id, AcceptedCase.judge), + AcceptedCase.capture(call_id, AcceptedCase.native), + SendBackCase.capture(call_id, SendBackCase.judge), + SendBackCase.capture(call_id, SendBackCase.native), + ) diff --git a/tests/test_commander_parity_verdicts.py b/tests/test_commander_parity_verdicts.py new file mode 100644 index 0000000000..ffa1f9db45 --- /dev/null +++ b/tests/test_commander_parity_verdicts.py @@ -0,0 +1,44 @@ +"""Observation invariants; the configured LifecycleRun journey supplies native proof.""" + +from dataclasses import replace + +from agent_comms.commander_parity.verdicts import ( + VerdictComparison, + VerdictMutation, + VerdictObservation, +) + + +def observation(mutation: VerdictMutation, changes: int) -> VerdictObservation: + return VerdictObservation("native verdict owner", 1, mutation.checks(changes), "", mutation) + + +def test_verdict_requires_the_actual_call_event_goal_and_strike_together(): + accepted = VerdictMutation("one-call", 1, 1, 0, 0, 0, "P1: Open · Accepted") + back = replace( + accepted, + strikes=1, + goal_revisions=1, + goal_history_entries=1, + progress="P1: Open · Sent back", + ) + ok, sent_back = observation(accepted, 0), observation(back, 1) + compared = VerdictComparison(ok, ok, sent_back, sent_back) + assert compared.passed + for changed in ( + replace(back, verdicts=0), + replace(back, todo_revisions=2), + replace(back, strikes=0), + replace(back, goal_revisions=0), + replace(back, goal_history_entries=2), + ): + observed = observation(changed, 1) + assert not observed.passed + assert not replace(compared, back_current=observed, back_native=observed).passed + for changed in ( + replace(sent_back, invocations=0), + replace(sent_back, checks=(*sent_back.checks, ("retained native identity", False))), + ): + assert not replace(compared, back_current=changed, back_native=changed).passed + for changed in (replace(back, call_id="other-call"), replace(back, progress="")): + assert not replace(compared, back_native=observation(changed, 1)).passed