From fcc03959b4faf3d9342fba3619cd35373fc1b61f Mon Sep 17 00:00:00 2001 From: Evan Gong Date: Fri, 16 Oct 2026 19:44:06 +0800 Subject: [PATCH 01/16] feat: add versioned strategic plans --- .../plans/2026-10-16-v3-issue-245.md | 99 +++++ docs/superpowers/plans/index.md | 1 + docs/tasks-and-automation.md | 2 + openkyrozen/agent/executor.py | 86 ++++- openkyrozen/agent/loop.py | 16 +- openkyrozen/agent/modes.py | 218 ++++++++++- openkyrozen/agent/prompts.py | 8 +- openkyrozen/persistence/store.py | 3 +- openkyrozen/persistence/tasks.py | 25 +- openkyrozen/tasks/engine.py | 71 +++- tests/test_execution_receipts.py | 68 ++++ tests/test_interaction.py | 364 +++++++++++++++++- 12 files changed, 922 insertions(+), 39 deletions(-) create mode 100644 docs/superpowers/plans/2026-10-16-v3-issue-245.md diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md new file mode 100644 index 0000000..73d3c27 --- /dev/null +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -0,0 +1,99 @@ +# V3 Issue #245 — Strategic Plans Implementation Plan + +> **For agentic workers:** execute this indexed plan task by task; keep the PR based on #242 and do not merge it. + +**Goal:** Preserve Plan Mode as a non-executing, versioned user approval of stable objectives, acceptance criteria, workspace scope, permitted effects/capabilities, and risk limits, with a linked mutable Working Plan. + +**Architecture:** Extend the existing `InteractionController` event-sourced proposal lifecycle and `TaskManager` transaction. Keep proposals and legacy message shapes as compatibility projections; persist the accepted Strategic Plan and linked Working Plan/tasks in the existing event and task stores, atomically. Strategic acceptance remains separate from capability grants, tool approvals, task evidence, and completion. + +**Tech Stack:** Python 3.12–3.13, unittest, existing SQLite repositories and interaction event log; no new dependency. + +**Spec:** GitHub issue [#245](https://github.com/EvanProgramming/OpenKyrozen/issues/245), requirements R069–R073, R077, and R080; approved batch plan in the task history. + +## Global Constraints + +- Plan Mode must not execute a proposal. +- Approval covers objectives, acceptance criteria, workspace paths, effect/capability categories, and the existing normal/high risk ceiling. +- Working Plan edits may adapt execution but cannot redefine approved intent. +- Plan approval never grants capabilities or replaces tool approval. +- Acceptance and its linked Working Plan/tasks/events commit atomically; duplicate acceptance is idempotent and stale versions fail without task creation. +- Preserve existing proposal/message shapes through explicit compatibility defaults; missing legacy bounds never widen scope automatically. +- No new dependency, paid provider call, or unrelated V3 work. + +## Files in Scope + +- `openkyrozen/agent/modes.py`: proposal validation, Strategic Plan version/fingerprint, controller state/acceptance, and user-facing rendering. +- `openkyrozen/agent/prompts.py`, `openkyrozen/agent/protocol.py`, `openkyrozen/agent/loop.py`: proposal contract, bounded defaults, acceptance and terminal lifecycle integration. +- `openkyrozen/tasks/engine.py`, `openkyrozen/tasks/ports.py`, `openkyrozen/persistence/tasks.py`, `openkyrozen/persistence/store.py`, and SQLite migration code: atomic creation of the linked Working Plan and event records, with per-task objective/criterion references. +- `tests/test_interaction.py`, `tests/test_working_plans.py`, `tests/test_sqlite_migrations.py`: contract, compatibility, lifecycle, and durability regressions. +- `docs/tasks-and-automation.md`, `docs/superpowers/plans/index.md`: user-facing contract and this indexed plan. + +## Review Focus + +- A stale or duplicate acceptance must never create duplicate tasks or accept a different version. +- A legacy proposal without bounds must not imply workspace or authority expansion. +- Revisions must keep stable objective/criterion identifiers where meaning is unchanged and create new identities when meaning changes. +- Failed persistence must leave both interaction state and Working Plan state unchanged. +- Completion and cancellation must remain distinct; cancellation cannot emit successful Strategic Plan completion. + +--- + +### Task 1: Versioned Strategic Plan contract + +**Files:** `openkyrozen/agent/modes.py`, `openkyrozen/agent/prompts.py`, `tests/test_interaction.py`. + +- [ ] Add failing tests for normalized bounds, duplicate objective/criterion identifiers, stable fingerprints, version increments, legacy proposals without bounds, and safe normal-risk/workspace defaults. +- [ ] Extend `validate_plan_proposal()` with `objectives`/step identity, stable acceptance criterion IDs, `bounds`, and a canonical fingerprint while preserving current `title`, `summary`, `assumptions`, `steps`, and `original_input` fields. +- [ ] Normalize legacy string criteria deterministically; preserve IDs across unchanged revisions and generate new criterion IDs when criterion meaning changes. +- [ ] Define compatibility defaults as the current workspace (`.`), read-only effect/capability, and normal risk; explicit proposal bounds may request more, but only the displayed user acceptance approves them. +- [ ] Update Plan Mode prompt examples and `render_plan()` to show objectives, criteria IDs, workspace paths, effects/capabilities, risk ceiling, version, and fingerprint. +- [ ] Run the focused proposal and rendering tests on Python 3.12 and 3.13. + +### Task 2: Atomic acceptance and task linkage + +**Files:** `openkyrozen/agent/modes.py`, `openkyrozen/tasks/engine.py`, `openkyrozen/tasks/ports.py`, `openkyrozen/persistence/store.py`, `openkyrozen/persistence/tasks.py`, migration code, `tests/test_interaction.py`, `tests/test_working_plans.py`, `tests/test_sqlite_migrations.py`. + +- [ ] Add failing tests for exact-version acceptance, duplicate acceptance, stale acceptance, and transaction rollback without partial plan/task/event state. +- [ ] Extend the existing Working Plan edit transaction to append the correlated `interaction.plan_accepted` event in the same SQLite transaction. +- [ ] Replace per-task `add_ordered_plan()` acceptance with one revision-zero Working Plan edit; attach Strategic Plan ID/version, objective ID, and acceptance criterion IDs to each task using existing checkpoint metadata. +- [ ] Persist a strategic binding on the Working Plan using the minimal SQLite field(s) needed for durable recovery; keep prior migration data intact. +- [ ] Reconstruct accepted and executing Strategic Plan state only from persisted proposal/acceptance events and the linked active Working Plan. Do not recreate tasks on restore. +- [ ] Run interaction, plan, and migration tests on Python 3.12 and 3.13. + +### Task 3: Lifecycle, interfaces, and compatibility + +**Files:** `openkyrozen/agent/modes.py`, `openkyrozen/agent/loop.py`, `openkyrozen/agent/protocol.py`, `openkyrozen/agent/session.py`, relevant interface projections, `tests/test_interaction.py`, `tests/test_web_sessions.py`, `docs/tasks-and-automation.md`. + +- [ ] Add failing tests for non-executing Plan Mode, linked task bounds, post-restart state, separate tool authorization, successful completion, and cancellation without false completion. +- [ ] Keep working-task adaptation tied to approved objective and acceptance IDs; do not allow acceptance to change workspace or execution authority. +- [ ] Emit one versioned definitive completion event only after existing evidence gates complete the linked Working Plan; terminal failure/cancellation must retain its actual status. +- [ ] Preserve legacy proposal payloads and direct callers through bounded compatibility adapters; ensure an unbounded historical proposal is marked legacy and cannot silently authorize scope expansion. +- [ ] Update docs to distinguish Strategic Plan approval, mutable Working Plan changes, and tool approvals. +- [ ] Run focused interface/session acceptance on Python 3.12 and 3.13. + +### Task 4: Verification and delivery + +- [ ] Run all focused tests on Python 3.12 and 3.13; run full tests, `make check`, `make lint`, `make docs-check`, Go TUI tests, and offline agent/subagent/TUI acceptance. +- [ ] Independently review the final diff, reproduce actionable Codex/CodeRabbit findings, publish fixes, and resolve applicable threads. +- [ ] GPG-sign commits and verify signatures locally and on GitHub; run final-head CI, attach the PR, and leave #245 open and unmerged. + +## Validation Record + +Implementation started from PR #284 head `0d49d4a488f6a9894b42900054cf06c665b9809f`, based on refreshed `main` `d71d2bd42b68485feb4571d3aad490d7d8327aa3`. + +- PASS: focused contract, atomic acceptance, stale-version, version-increment, capability/effect/risk, workspace-path and symlink-scope tests on Python 3.12 and 3.13. +- PASS: `make check`, `make lint`, and Python compile checks. +- BLOCKED: `make docs-check` and the app-bootstrap portions of the full unittest suite import the installed Python 3.14 Pydantic binary through `PYTHONPATH`; this binary is unavailable to the local 3.12/3.13 interpreters. Full unittest run: 647 tests, 7 skipped, 11 failures and 259 errors, dominated by that import failure. CI on the supported runtimes is required to verify the complete suite. +- PENDING: independent diff review, CI, signed commit verification, and PR publication. + +## Review follow-up + +- [x] Reproduced the runtime receipt path for a two-criterion objective. The first verified receipt previously succeeded the task, while a generic checkpoint action also shadowed later criteria with different actions. +- [x] Strategic task receipts now preserve per-criterion matching and keep the task active until all bound acceptance IDs have verified evidence. +- [x] Added a runtime receipt regression that processes the actual write and read receipts in sequence, confirms the task remains active after the first receipt, and succeeds after the second. +- [x] Rechecked the subagent file-verification review findings against current `subagent_runtime.py` and its content-matching regression; both behaviors were already implemented in the base commit. +- PASS: strategic plan acceptance suite, 10 tests, on Python 3.12 and 3.13. +- BLOCKED: runtime application bootstrap tests remain blocked locally by missing Python 3.14 `pydantic_core`; no paid provider calls were used. +- PASS: `make lint` and `git diff --check` after review fixes. +- BLOCKED: `make check` passed Python architecture, syntax, inventory, and shell checks but the Go test exhausted the host's 117 MiB remaining disk. The Go cache was cleared; rerun is pending. +- BLOCKED: `make docs-check` reaches the inventory generator but the installed Python 3.14 `pydantic_core` extension cannot load under Python 3.12. diff --git a/docs/superpowers/plans/index.md b/docs/superpowers/plans/index.md index a59396d..f82576f 100644 --- a/docs/superpowers/plans/index.md +++ b/docs/superpowers/plans/index.md @@ -11,6 +11,7 @@ - [V3 #264 — Version SQLite state and migrate safely](2026-10-14-v3-issue-264.md) - [V3 #241 — Working Plans and task lifecycle](2026-10-14-v3-issue-241.md) - [V3 #242 — Atomic Working Plan edits](2026-10-16-v3-issue-242.md) +- [V3 #245 — Versioned, user-approved Strategic Plans](2026-10-16-v3-issue-245.md) - [V3 #236 — Execution receipts and task association](2026-10-14-v3-issue-236.md) - [V3 #255 — Shared typed runtime events](2026-10-14-v3-issue-255.md) diff --git a/docs/tasks-and-automation.md b/docs/tasks-and-automation.md index 4eb7169..9360a19 100644 --- a/docs/tasks-and-automation.md +++ b/docs/tasks-and-automation.md @@ -13,6 +13,8 @@ Task IDs and records are actor/workspace/session scoped. The web task API expose Simple work does not require a plan, and a plan is optional for medium work. Complex direct work uses a durable Working Plan with runtime-owned stable IDs and explicit acceptance criteria. The typed `update_plan` boundary applies revision-checked batches of add, update, cancel, reorder, and dependency operations; legacy `complexity`/`tasks` input is adapted through the same validation path. A changed task must still be pending and unexecuted; executed task records and evidence are preserved, and changed work is represented by a new task after cancellation. Scheduling follows plan order and waits for every dependency to succeed. A plan completes only when every task succeeds with acceptance evidence; recovery also reconciles a crash between the final task write and the plan status write. Clearing an active plan cancels its unfinished steps. The existing foreground text protocol continues to use Plan/TaskList compatibility until the native foreground loop is migrated. +Plan Mode can produce a versioned Strategic Plan before execution. Acceptance records the exact objectives, acceptance criteria, workspace paths, capabilities, effect categories, and risk ceiling alongside its linked Working Plan. Each criterion should include a `verification` action and exact object arguments; only a successful receipt matching that contract records the criterion ID as verified. Plain-text criteria remain readable for legacy proposals but cannot satisfy the per-criterion completion gate without a verifier. Runtime tool dispatch checks each tool's catalog policy and supplied paths against those accepted bounds; shell processes are denied because this runtime cannot confine arbitrary commands to those paths. An unbounded legacy proposal receives read-only defaults. Strategic acceptance does not grant capabilities, bypass tool approvals, or establish completion evidence. + For a typed plan step, include an optional `execution` object with the canonical tool `action` and object `arguments`. Native tool calls associate with a task only when the runtime receives that task's explicit ID; matching action and arguments alone never select a task. A successful associated receipt may add execution evidence, but it cannot satisfy acceptance criteria by itself. A delegated assignment may carry that explicit Working Plan task ID; the parent records task evidence only after the independent review succeeds. The legacy text adapter retains its bounded matching behavior for compatibility. Plan IDs and task IDs are returned as structured result data so later updates can retain runtime-owned task identities. ## Sub-agents diff --git a/openkyrozen/agent/executor.py b/openkyrozen/agent/executor.py index dff7ed7..e3791a6 100644 --- a/openkyrozen/agent/executor.py +++ b/openkyrozen/agent/executor.py @@ -6,6 +6,7 @@ import re import uuid import time +from pathlib import Path from typing import Any from openkyrozen.persistence.models import stable_hash, utc_now from openkyrozen.app.config import AgentConfigError, effective_capabilities, load_agent_config @@ -17,6 +18,50 @@ from openkyrozen.tasks.models import _receipt_args +def _strategic_plan_denial(plan, action, args, spec, workspace_root): + """Return a denial when a tool exceeds the bounds the user accepted.""" + bounds = plan.get("bounds") if isinstance(plan, dict) else None + if not isinstance(bounds, dict): + return "accepted plan has no enforceable bounds" + capabilities = set(bounds.get("capabilities", ())) + effects = set(bounds.get("effects", ())) + if not spec or spec.capability not in capabilities: + return "tool capability is outside the accepted plan" + effect = { + "none": "read_only", "read": "read_only", "read_only": "read_only", + "mutation": "filesystem", "internal_write": "internal", "orchestration": "internal", + "stateful": "internal", "external": "remote", "unknown": "unknown", + }.get(spec.side_effects, spec.side_effects) + if effect not in effects: + return "tool effect is outside the accepted plan" + if effect == "process": + return "process effects cannot be confined to approved workspace paths" + if spec.risk == "high" and bounds.get("risk_ceiling") != "high": + return "tool risk exceeds the accepted plan" + + paths = {} + if isinstance(args, dict): + paths = {key: value for key, value in args.items() + if key in {"path", "file_path", "directory", "destination", "target_path", "clone_path"} + and isinstance(value, str) and value.strip()} + elif action in {"write_file", "read_file", "edit_file", "search_files", "list_dir", "list_tree"}: + raw = str(args).split("|", 1)[0].strip() + if raw: + paths = {"path": raw} + if effect == "filesystem" and not paths: + return "filesystem effect has no path that can be checked against the accepted scope" + root = Path(workspace_root).expanduser().resolve() + allowed_roots = [(root / str(path)).resolve() for path in bounds.get("workspace_paths", ())] + if any(allowed != root and root not in allowed.parents for allowed in allowed_roots): + return "accepted workspace scope resolves outside the active workspace" + for value in paths.values(): + candidate = Path(value).expanduser() + candidate = (candidate if candidate.is_absolute() else root / candidate).resolve() + if not any(candidate == allowed or allowed in candidate.parents for allowed in allowed_roots): + return "tool path is outside the accepted workspace scope" + return None + + def _notify_tool_execute(self, action: str, args: Any, result: ToolResult) -> None: """Emit the typed outcome to observers; display text cannot change status.""" result = result if isinstance(result, ToolResult) else tool_failure( @@ -241,6 +286,15 @@ def finish(outcome: ToolResult, authorized: bool = False, raw_result: Any = None outcome = tool_failure("invalid_arguments", f"Error: unknown tool '{action}'") self._notify_tool_execute(action, args, outcome) return finish(outcome) + executing_plan = self._interaction_controller.state().get("executing_plan") + if executing_plan: + denial = _strategic_plan_denial( + executing_plan, action, args, self.tool_registry.specs.get(action), self._get_workspace_root(), + ) + if denial: + outcome = tool_failure("permission_denied", f"Error: {denial}.") + self._notify_tool_execute(action, args, outcome) + return finish(outcome) from openkyrozen.security.permission_gate import git_read_arguments_allowed if not git_read_arguments_allowed(action, args): outcome = tool_failure("invalid_arguments", "Error: read-only Git tools reject output-writing and external-execution options.") @@ -291,6 +345,19 @@ def finish(outcome: ToolResult, authorized: bool = False, raw_result: Any = None and request.session_id == self.current_session.session_id and self._execution_capability_token.allows(current_spec.capability) ) + if authorized and executing_plan: + current_plan = self._interaction_controller.state().get("executing_plan") + if (not current_plan or current_plan.get("plan_id") != executing_plan.get("plan_id") + or current_plan.get("fingerprint") != executing_plan.get("fingerprint")): + denial = "accepted plan changed while approval was pending" + else: + denial = _strategic_plan_denial( + current_plan, action, args, self.tool_registry.specs.get(action), self._get_workspace_root(), + ) + if denial: + outcome = tool_failure("permission_denied", f"Error: {denial}.") + self._notify_tool_execute(action, args, outcome) + return finish(outcome) if not authorized: outcome = tool_failure("approval_denied", f"Error: {action} requires confirmation. Approve it interactively or set KYROZEN_APPROVAL_MODE=never for an explicitly automated CLI.") self._notify_tool_execute(action, args, outcome) @@ -423,11 +490,15 @@ def _record_turn_receipt(self, receipt: ExecutionReceipt, *, allow_legacy_task_m bool(explicit_task_id) or receipt.call_id_source == "legacy" or (allow_legacy_task_matching and receipt.call_id_source == "synthetic") ) + matching_args = (json.dumps(receipt.arguments, ensure_ascii=False, allow_nan=False, + sort_keys=True, separators=(",", ":")) + if receipt.arguments is not None else receipt.args) evidence = {} if receipt.action in TOOLS or not should_reconcile else self.tasks.record_evidence( - task_id=explicit_task_id, action=receipt.action, args=receipt.args, + task_id=explicit_task_id, action=receipt.action, args=matching_args, args_projection=receipt_projection["args"], args_fingerprint=receipt.args_sha256, result=receipt_projection["result"], success=receipt.success, acceptance=task_acceptance, receipt_id=receipt.receipt_id, + verification_status=receipt.verification.get("status"), ) task_id = evidence.get("task_id") effective_acceptance = evidence.get("acceptance") or ( @@ -435,11 +506,18 @@ def _record_turn_receipt(self, receipt: ExecutionReceipt, *, allow_legacy_task_m ) if task_id: task = next((item for item in self.tasks.tasks if item["id"] == task_id), None) - if task and not (task.get("checkpoint") or {}).get("action"): + checkpoint = (task or {}).get("checkpoint") or {} + if task and not checkpoint.get("action") and not checkpoint.get("strategic_plan"): self.tasks.update_checkpoint(task_id, {"action": receipt.action, "args": receipt_projection["args"]}) if task_id and receipt.success and effective_acceptance: - index = next(index for index, task in enumerate(self.tasks.tasks) if task["id"] == task_id) - self.tasks.set_status(index, "succeeded") + task = next((item for item in self.tasks.tasks if item["id"] == task_id), None) + strategic = (task or {}).get("checkpoint", {}).get("strategic_plan", {}) + required = set(strategic.get("acceptance_ids", [])) + covered = {criterion_id for item in (task or {}).get("evidence", []) + for criterion_id in item.get("acceptance_ids", [])} + if not required or required <= covered: + index = next(index for index, item in enumerate(self.tasks.tasks) if item["id"] == task_id) + self.tasks.set_status(index, "succeeded") receipt_payload = receipt_projection if receipt.task_id is not None and receipt.task_id != task_id: receipt_payload["requested_task_id"] = receipt.task_id diff --git a/openkyrozen/agent/loop.py b/openkyrozen/agent/loop.py index 3567a51..4f67412 100644 --- a/openkyrozen/agent/loop.py +++ b/openkyrozen/agent/loop.py @@ -2,7 +2,7 @@ import json from typing import Any -from openkyrozen.tasks.engine import is_complete, is_terminal +from openkyrozen.tasks.engine import is_terminal from openkyrozen.agent.modes import InteractionError, is_plan_acceptance from openkyrozen.workspace.history_port import TurnToken @@ -108,10 +108,16 @@ def _chat_turn(self, user_input: str, clear_tasks: bool = False, profile: str | error=f"{type(exc).__name__}: {exc}", **context) raise state_after = self._interaction_controller.state() - if (active_plan and not state_after.get("pending_question") and self.tasks.tasks - and all(is_terminal(task) for task in self.tasks.tasks)): - status = "completed" if all(is_complete(task) for task in self.tasks.tasks) else "failed" - self._interaction_controller.complete_plan(active_plan, status=status) + active_task_ids = active_plan.get("working_task_ids", []) if active_plan else [] + active_tasks = [task for task in self.tasks.tasks if task.get("id") in active_task_ids] + if (active_plan and active_task_ids and not state_after.get("pending_question") and active_tasks + and all(is_terminal(task) for task in active_tasks)): + statuses = {task.get("status") for task in active_tasks} + status = ("completed" if statuses == {"succeeded"} else + "cancelled" if statuses <= {"succeeded", "cancelled"} else "failed") + self._interaction_controller.complete_plan( + active_plan, status=status, task_manager=self.tasks, + ) self._emit_stream_event({"event": "interaction", "interaction": self.interaction_envelope()}) self._track_fix_outcome(original_user_input, reply) runtime.turn_end(reply=reply, success=True, **context) diff --git a/openkyrozen/agent/modes.py b/openkyrozen/agent/modes.py index 31ace91..293f594 100644 --- a/openkyrozen/agent/modes.py +++ b/openkyrozen/agent/modes.py @@ -3,6 +3,8 @@ from __future__ import annotations import json +import hashlib +import posixpath import re import shlex import uuid @@ -187,18 +189,102 @@ def validate_plan_proposal(value: Any, *, previous: dict[str, Any] | None = None acceptance = [acceptance] if not isinstance(acceptance, list) or not 1 <= len(acceptance) <= 5: raise InteractionError("each plan step requires 1-5 acceptance criteria") + acceptance_verifications = [] + normalized_acceptance = [] + for item in acceptance: + verification = None + if isinstance(item, dict) and set(item) == {"criterion", "verification"}: + verification = item["verification"] + if (not isinstance(verification, dict) or set(verification) != {"action", "arguments"} + or not isinstance(verification.get("action"), str) + or not verification["action"].strip() or not isinstance(verification.get("arguments"), dict)): + raise InteractionError("acceptance verification must contain an action and object arguments") + try: + json.dumps(verification["arguments"], ensure_ascii=False, allow_nan=False) + except (TypeError, ValueError) as exc: + raise InteractionError("acceptance verification arguments must be JSON-safe") from exc + criterion = _text(item.get("criterion") if isinstance(item, dict) else item, + "acceptance criterion", 300) + normalized_acceptance.append(criterion) + acceptance_verifications.append(verification) + acceptance = normalized_acceptance + acceptance_ids = [] + for criterion, verification in zip(acceptance, acceptance_verifications): + identity = json.dumps({"criterion": criterion, "verification": verification}, sort_keys=True, + ensure_ascii=False, separators=(",", ":")) + acceptance_ids.append(f"{step_id}-ac-{hashlib.sha256(identity.encode('utf-8')).hexdigest()[:12]}") + if len(set(acceptance_ids)) != len(acceptance_ids): + raise InteractionError("acceptance criteria must be unique within each objective") normalised.append({ "id": step_id, "title": _text(step.get("title") or f"Step {index}", "step title", 120), "description": _text(step.get("description"), "step description", 800), - "acceptance": [_text(item, "acceptance criterion", 300) for item in acceptance], + "acceptance": acceptance, + "acceptance_ids": acceptance_ids, + "acceptance_verifications": acceptance_verifications, }) assumptions = value.get("assumptions", []) if assumptions is None: assumptions = [] if not isinstance(assumptions, list) or len(assumptions) > 10: raise InteractionError("assumptions must contain at most 10 items") - return { + raw_bounds = value.get("bounds") + bounds_source = "proposal" + if raw_bounds is None: + if "bounds" in value: + raise InteractionError("bounds must be an object") + raw_bounds = { + "workspace_paths": ["."], "capabilities": ["read"], + "effects": ["read_only"], "risk_ceiling": "normal", + } + bounds_source = "legacy-default" + if not isinstance(raw_bounds, dict) or set(raw_bounds) - { + "workspace_paths", "capabilities", "effects", "risk_ceiling", + }: + raise InteractionError("bounds must declare workspace paths, capabilities, effects, and risk ceiling") + + def values(field: str, allowed: set[str] | None = None) -> list[str]: + raw = raw_bounds.get(field, []) + if not isinstance(raw, list) or len(raw) > 32: + raise InteractionError(f"bounds.{field} must be an array with at most 32 values") + result = [] + for item in raw: + if not isinstance(item, str) or not item.strip(): + raise InteractionError(f"bounds.{field} entries must be nonempty strings") + item = item.strip() + if allowed is not None and item not in allowed: + raise InteractionError(f"unsupported bounds.{field} value: {item}") + result.append(item) + if len(set(result)) != len(result): + raise InteractionError(f"bounds.{field} values must be unique") + return result + + paths = values("workspace_paths") + normalised_paths = [] + for path in paths: + path = path.replace("\\", "/") + if path.startswith("/") or re.match(r"^[A-Za-z]:", path): + raise InteractionError("workspace paths must be relative to the active workspace") + path = posixpath.normpath(path) + if path == ".." or path.startswith("../"): + raise InteractionError("workspace paths cannot escape the active workspace") + normalised_paths.append(path) + if len(set(normalised_paths)) != len(normalised_paths): + raise InteractionError("workspace paths must be unique after normalization") + capabilities = values("capabilities", { + "read", "write", "shell", "network", "git", "browser", "destructive", "dynamic", + }) + effects = values("effects", { + "read_only", "filesystem", "process", "remote", "destructive", "internal", + }) + risk_ceiling = raw_bounds.get("risk_ceiling", "normal") + if risk_ceiling not in {"normal", "high"}: + raise InteractionError("risk ceiling must be normal or high") + bounds = { + "workspace_paths": sorted(normalised_paths), "capabilities": sorted(capabilities), + "effects": sorted(effects), "risk_ceiling": risk_ceiling, + } + plan = { "plan_id": _text( (previous or {}).get("plan_id") or f"plan_{uuid.uuid4().hex}", "plan_id", 100, @@ -208,8 +294,18 @@ def validate_plan_proposal(value: Any, *, previous: dict[str, Any] | None = None "summary": _text(value.get("summary"), "plan summary", 1200), "assumptions": [_text(item, "assumption", 300) for item in assumptions], "steps": normalised, + "bounds": bounds, + "bounds_source": bounds_source, "original_input": str(original_input).strip()[:12_000], } + fingerprint_fields = {key: plan[key] for key in ( + "plan_id", "version", "title", "summary", "assumptions", "steps", "bounds", + )} + plan["fingerprint"] = hashlib.sha256(json.dumps( + fingerprint_fields, ensure_ascii=False, allow_nan=False, sort_keys=True, + separators=(",", ":"), + ).encode("utf-8")).hexdigest() + return plan def parse_control_block(text: str, name: str) -> Any | None: @@ -370,10 +466,12 @@ def state(self, user_input: str = "") -> dict[str, Any]: question = None elif event_type == "interaction.plan_proposed": plan = payload - executing = None elif event_type == "interaction.plan_accepted": if plan and payload.get("plan_id") == plan.get("plan_id") and payload.get("version") == plan.get("version"): executing = dict(plan) + for field in ("working_plan_id", "working_plan_revision", "working_task_ids"): + if field in payload: + executing[field] = payload[field] plan = None elif event_type == "interaction.plan_cancelled": if plan and payload.get("plan_id") == plan.get("plan_id"): @@ -451,7 +549,8 @@ def reopen_question(self) -> dict[str, Any]: return question def propose_plan(self, value: Any, *, original_input: str = "") -> dict[str, Any]: - previous = self.state().get("pending_plan") + state = self.state() + previous = state.get("pending_plan") or state.get("executing_plan") plan = validate_plan_proposal(value, previous=previous, original_input=original_input) self._append("interaction.plan_proposed", plan) return plan @@ -465,43 +564,104 @@ def cancel_plan(self, plan_id: str | None = None, version: int | None = None) -> def accepted_plan(self, plan_id: str | None = None, version: int | None = None) -> dict[str, Any] | None: proposals: dict[tuple[str, int], dict[str, Any]] = {} - accepted: set[tuple[str, int]] = set() + accepted: dict[tuple[str, int], dict[str, Any]] = {} for event in self._history(): payload = event.get("payload", {}) key = (str(payload.get("plan_id") or ""), int(payload.get("version") or 0)) if event["event_type"] == "interaction.plan_proposed": proposals[key] = payload elif event["event_type"] == "interaction.plan_accepted": - accepted.add(key) + accepted[key] = payload matches = [proposal for key, proposal in proposals.items() if key in accepted and (not plan_id or key[0] == plan_id) and (version is None or key[1] == version)] - return matches[-1] if matches else None + if not matches: + return None + proposal = matches[-1] + key = (str(proposal.get("plan_id") or ""), int(proposal.get("version") or 0)) + return {**proposal, **{field: accepted[key][field] for field in ( + "working_plan_id", "working_plan_revision", "working_task_ids", + ) if field in accepted[key]}} def accept_plan(self, task_manager: Any, *, plan_id: str | None = None, version: int | None = None) -> tuple[dict[str, Any], bool]: state = self.state() plan = state.get("pending_plan") executing = state.get("executing_plan") - if executing and (not plan_id or plan_id == executing.get("plan_id")): + if executing and not plan and (not plan_id or plan_id == executing.get("plan_id")): + if version is not None and version != executing.get("version"): + raise InteractionError("accepted plan version is stale") return executing, False + if plan and executing: + raise InteractionError("a revised Strategic Plan requires reapproval before replacing the active plan") if not plan or (plan_id and plan_id != plan.get("plan_id")) or (version is not None and version != plan.get("version")): accepted = self.accepted_plan(plan_id, version) if accepted: return accepted, False raise InteractionError("plan is not pending") - descriptions = [f"{step['title']}: {step['description']}" for step in plan["steps"]] - acceptance = [ - [{"criterion": criterion} for criterion in step["acceptance"]] - for step in plan["steps"] - ] - prefix = f"{plan['plan_id']}-v{plan['version']}" - task_manager.add_ordered_plan(descriptions, task_id_prefix=prefix, acceptance=acceptance) - self._append("interaction.plan_accepted", {"plan_id": plan["plan_id"], "version": plan["version"]}) - return plan, True - - def complete_plan(self, plan: dict[str, Any], *, status: str = "completed") -> None: + operations = [] + strategic_bindings = {} + for step in plan["steps"]: + operations.append({"op": "add", "ref": step["id"], "task": { + "description": f"{step['title']}: {step['description']}", + "acceptance": step["acceptance"], + }}) + strategic_bindings[step["id"]] = { + "plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"], "objective_id": step["id"], + "acceptance_ids": step["acceptance_ids"], + "acceptance_verifications": step["acceptance_verifications"], + } + working = task_manager.edit_plan( + complexity="complex", expected_revision=0, operations=operations, + strategic_bindings=strategic_bindings, + transaction_event={"event_type": "interaction.plan_accepted", "payload": { + "plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"], + }, "expected_proposal": { + "plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan["fingerprint"], + }}, + ) + return {**plan, "working_plan_id": working["plan_id"], + "working_plan_revision": working["revision"], + "working_task_ids": [task["id"] for task in working["tasks"]]}, True + + def complete_plan(self, plan: dict[str, Any], *, status: str = "completed", + task_manager: Any | None = None) -> None: + if status not in {"completed", "failed", "cancelled"}: + raise InteractionError("Strategic Plan terminal status is invalid") + if any(event["event_type"] == "interaction.plan_completed" + and event.get("payload", {}).get("plan_id") == plan.get("plan_id") + and event.get("payload", {}).get("version") == plan.get("version") + for event in self._history()): + return + task_ids = plan.get("working_task_ids") + if status == "completed": + if task_manager is None or not isinstance(task_ids, list) or not task_ids: + raise InteractionError("Strategic Plan completion requires its linked Working Plan evidence") + tasks = {task.get("id"): task for task in task_manager.tasks} + if any(tasks.get(task_id, {}).get("status") != "succeeded" + or not any(isinstance(item, dict) and item.get("success") is True and item.get("acceptance") + for item in tasks[task_id].get("evidence", [])) + for task_id in task_ids): + raise InteractionError("Strategic Plan tasks have not passed their acceptance evidence gates") + for task_id in task_ids: + task = tasks[task_id] + binding = (task.get("checkpoint") or {}).get("strategic_plan") or {} + required = set(binding.get("acceptance_ids", ())) + if required: + covered = { + criterion_id + for item in task.get("evidence", []) + if isinstance(item, dict) and item.get("success") is True + for criterion_id in item.get("acceptance_ids", ()) + } + if not required <= covered: + raise InteractionError("Strategic Plan criteria have not all been verified") self._append("interaction.plan_completed", { - "plan_id": plan["plan_id"], "version": plan["version"], "status": status, + "plan_id": plan["plan_id"], "version": plan["version"], + "fingerprint": plan.get("fingerprint"), "working_plan_id": plan.get("working_plan_id"), + "working_task_ids": task_ids or [], "status": status, }) @@ -523,6 +683,22 @@ def render_plan(plan: dict[str, Any]) -> str: lines.extend(["", "Plan:"]) for index, step in enumerate(plan["steps"], 1): lines.append(f"{index}. {step['title']} — {step['description']}") - lines.extend(f" - Acceptance: {item}" for item in step["acceptance"]) + lines.extend(f" - Acceptance [{criterion_id}]: {item}" + for criterion_id, item in zip(step["acceptance_ids"], step["acceptance"])) + for verification in step["acceptance_verifications"]: + if verification: + lines.append(f" Verified by {verification['action']} with the approved arguments.") + else: + lines.append(" No verification contract; this criterion cannot pass the completion gate.") + bounds = plan["bounds"] + lines.extend([ + "", "Approved bounds:", + f"- Workspace paths: {', '.join(bounds['workspace_paths']) or '(none; no file paths)' }", + f"- Capabilities: {', '.join(bounds['capabilities']) or '(none)' }", + f"- Effects: {', '.join(bounds['effects']) or '(none)' }", + f"- Risk ceiling: {bounds['risk_ceiling']}", + f"- Bounds provenance: {plan.get('bounds_source', 'legacy-default')}", + f"- Strategic fingerprint: {plan['fingerprint']}", + ]) lines.extend(["", "Accept with `/plan accept` or `accept plan`; send feedback to revise; cancel with `/plan cancel`."]) return "\n".join(lines) diff --git a/openkyrozen/agent/prompts.py b/openkyrozen/agent/prompts.py index 72e1ff2..ceb57fb 100644 --- a/openkyrozen/agent/prompts.py +++ b/openkyrozen/agent/prompts.py @@ -153,9 +153,9 @@ def _system_prompt(self, tools_list: str, agent_config: dict[str, Any] | None = "In `ask` mode, answer or investigate with read/network tools only.\n" "In `plan` mode, first use exactly one read-only inspection Action, then output only a PlanProposal block. Do not execute the plan:\n" "PlanProposal:\n```json\n" - '{"title":"Plan title","summary":"Outcome","assumptions":[],"steps":[{"id":"step-1","title":"Step","description":"Work to perform","acceptance":["Observable result"]}]}\n' + '{"title":"Plan title","summary":"Outcome","assumptions":[],"bounds":{"workspace_paths":["src"],"capabilities":["read","write"],"effects":["read_only","filesystem"],"risk_ceiling":"normal"},"steps":[{"id":"objective-1","title":"Step","description":"Work to perform","acceptance":[{"criterion":"File contains the requested text","verification":{"action":"read_file","arguments":{"path":"src/example.txt"}}}]}]}\n' "```\n" - "Use 1-10 stable step IDs. A revision is a new version created by the runtime.\n" + "Use 1-10 stable objective IDs and list exact workspace-relative paths, capability/effect categories, and a normal or high risk ceiling in bounds. Give each acceptance criterion a verification action and exact arguments; only matching successful receipts prove that criterion. A revision is a new version created by the runtime.\n" "In `agent` mode, execute within the available capabilities and existing approval policy.\n" "Legacy Plan and TaskList execution applies only in `agent` mode.\n" ) @@ -197,8 +197,8 @@ def _system_prompt(self, tools_list: str, agent_config: dict[str, Any] | None = + ( "The final plan must use this standalone control:\n" "PlanProposal:\n```json\n" - '{"title":"Plan title","summary":"Outcome","assumptions":[],"steps":[{"id":"step-1","title":"Step","description":"Work to perform after acceptance","acceptance":["Observable result"]}]}\n' - "```\nUse 1-10 stable step IDs. Output no task or execution controls.\n" + '{"title":"Plan title","summary":"Outcome","assumptions":[],"bounds":{"workspace_paths":["src"],"capabilities":["read","write"],"effects":["read_only","filesystem"],"risk_ceiling":"normal"},"steps":[{"id":"objective-1","title":"Step","description":"Work to perform after acceptance","acceptance":["Observable result"]}]}\n' + "```\nUse 1-10 stable objective IDs and state the smallest workspace paths, capability/effect categories, and normal or high risk ceiling. Output no task or execution controls.\n" if interaction_mode == "plan" else "" ) + "\n## Available read-only tools\n" + readonly_tools + "\n\n" diff --git a/openkyrozen/persistence/store.py b/openkyrozen/persistence/store.py index 98132ba..745f120 100644 --- a/openkyrozen/persistence/store.py +++ b/openkyrozen/persistence/store.py @@ -178,10 +178,11 @@ def write_working_plan(self, plan, tasks, *, user_id, workspace_id, session_id): ) def apply_working_plan_edit(self, plan, tasks, *, expected_revision, expected_tasks, - user_id, workspace_id, session_id): + user_id, workspace_id, session_id, transaction_event=None): return self.tasks.apply_working_plan_edit( plan, tasks, expected_revision=expected_revision, expected_tasks=expected_tasks, user_id=user_id, workspace_id=workspace_id, session_id=session_id, + transaction_event=transaction_event, ) def claim_task(self, *, user_id, workspace_id, session_id): diff --git a/openkyrozen/persistence/tasks.py b/openkyrozen/persistence/tasks.py index d50aee9..9472202 100644 --- a/openkyrozen/persistence/tasks.py +++ b/openkyrozen/persistence/tasks.py @@ -208,7 +208,8 @@ def write_working_plan(self, plan: dict[str, Any], tasks: list[dict[str, Any]], def apply_working_plan_edit(self, plan: dict[str, Any], tasks: list[dict[str, Any]], *, expected_revision: int, expected_tasks: list[dict[str, Any]], - user_id: str, workspace_id: str, session_id: str | None) -> int: + user_id: str, workspace_id: str, session_id: str | None, + transaction_event: dict[str, Any] | None = None) -> int: """Commit a validated plan snapshot only if its revision and execution state are current.""" import uuid @@ -217,6 +218,20 @@ def apply_working_plan_edit(self, plan: dict[str, Any], tasks: list[dict[str, An with self.database._lock, self.database.connection() as db: db.execute("BEGIN IMMEDIATE") try: + expected_proposal = (transaction_event or {}).get("expected_proposal") + if expected_proposal is not None: + latest = db.execute( + "SELECT event_type,payload FROM events WHERE user_id=? AND workspace_id=? " + "AND session_id=? AND event_type IN ('interaction.plan_proposed', " + "'interaction.plan_accepted', 'interaction.plan_cancelled') " + "ORDER BY created_at DESC,rowid DESC LIMIT 1", + (user_id, workspace_id, scoped_session), + ).fetchone() + latest_payload = self.database._loads(latest["payload"], {}) if latest else {} + if (not latest or latest["event_type"] != "interaction.plan_proposed" + or any(latest_payload.get(key) != value + for key, value in expected_proposal.items())): + raise ValueError("Strategic Plan proposal changed; reload before accepting") current = db.execute( "SELECT * FROM working_plans WHERE user_id=? AND workspace_id=? AND session_id=? AND status='active'", (user_id, workspace_id, scoped_session), @@ -294,6 +309,14 @@ def progress(rows): "working_plan.cancelled" if plan["status"] == "cancelled" else "working_plan.updated", self.database._json(event_payload), user_id, workspace_id, session_id, now), ) + if transaction_event is not None: + db.execute( + "INSERT INTO events(id,event_type,payload,user_id,workspace_id,session_id,task_id,created_at) " + "VALUES(?,?,?,?,?,?,NULL,?)", + (f"evt_{uuid.uuid4().hex}", transaction_event["event_type"], + self.database._json(transaction_event["payload"]), + user_id, workspace_id, scoped_session, now), + ) db.commit() return revision except Exception: diff --git a/openkyrozen/tasks/engine.py b/openkyrozen/tasks/engine.py index aabbe88..3fa78c0 100644 --- a/openkyrozen/tasks/engine.py +++ b/openkyrozen/tasks/engine.py @@ -179,7 +179,9 @@ def _plan_task_fields(task: Any) -> dict[str, Any]: "dependencies": list(dependencies), "priority": priority, "checkpoint": checkpoint} def edit_plan(self, *, expected_revision: int, operations: list[dict[str, Any]], - complexity: str | None = None, plan_id: str | None = None) -> dict[str, Any]: + complexity: str | None = None, plan_id: str | None = None, + strategic_bindings: dict[str, dict[str, Any]] | None = None, + transaction_event: dict[str, Any] | None = None) -> dict[str, Any]: """Apply a validated ordered operation batch as one revision-checked transaction.""" if type(expected_revision) is not int or expected_revision < 0: raise ValueError("expected_revision must be a nonnegative integer") @@ -189,6 +191,20 @@ def edit_plan(self, *, expected_revision: int, operations: list[dict[str, Any]], raise ValueError("operations cannot be empty") if plan_id is not None and (not isinstance(plan_id, str) or not plan_id.strip()): raise ValueError("plan_id must be a nonempty string") + if strategic_bindings is not None and not isinstance(strategic_bindings, dict): + raise ValueError("strategic_bindings must be an object") + if transaction_event is not None and ( + not isinstance(transaction_event, dict) + or set(transaction_event) not in ( + {"event_type", "payload"}, {"event_type", "payload", "expected_proposal"}, + ) + or transaction_event.get("event_type") != "interaction.plan_accepted" + or not isinstance(transaction_event.get("payload"), dict) + or ("expected_proposal" in transaction_event and ( + not isinstance(transaction_event["expected_proposal"], dict) + or set(transaction_event["expected_proposal"]) != {"plan_id", "version", "fingerprint"} + ))): + raise ValueError("only a structured plan-acceptance transaction event is supported") saved = self.store.get_working_plan( user_id=self.user_id, workspace_id=self.workspace_id, session_id=self.session_id or "", @@ -216,6 +232,7 @@ def edit_plan(self, *, expected_revision: int, operations: list[dict[str, Any]], tasks = deepcopy(previous_tasks) by_id = {task["id"]: task for task in tasks} references: dict[str, str] = {} + used_strategic_bindings: set[str] = set() now = utc_now() scope_key = _task_key(f"{self.user_id}:{self.workspace_id}:{self.session_id or ''}") @@ -250,6 +267,35 @@ def resolve(value: Any) -> str: task = {"id": task_id, **fields, "status": "pending", "attempts": 0, "checkpoint": fields["checkpoint"], "evidence": [], "created_at": now, "updated_at": now, "plan_id": plan_id} + if strategic_bindings and ref in strategic_bindings: + binding = strategic_bindings[ref] + if (not isinstance(binding, dict) + or set(binding) != {"plan_id", "version", "fingerprint", "objective_id", "acceptance_ids", + "acceptance_verifications"} + or not isinstance(binding.get("version"), int) + or not isinstance(binding.get("acceptance_ids"), list) + or not isinstance(binding.get("acceptance_verifications"), list)): + raise ValueError("strategic task binding is invalid") + task["checkpoint"] = {**task["checkpoint"], "strategic_plan": deepcopy(binding)} + verifications = binding["acceptance_verifications"] + if len(verifications) != len(fields["acceptance"]): + raise ValueError("strategic acceptance verifications do not match the task") + task["acceptance"] = [] + for criterion_id, criterion, verification in zip( + binding["acceptance_ids"], fields["acceptance"], verifications): + item = {"id": criterion_id, "criterion": criterion} + if verification is not None: + if (not isinstance(verification, dict) or set(verification) != {"action", "arguments"} + or not isinstance(verification.get("action"), str) + or not isinstance(verification.get("arguments"), dict)): + raise ValueError("strategic acceptance verification is invalid") + item["action"] = _receipt_action(verification["action"]) + item["args"] = json.dumps(verification["arguments"], ensure_ascii=False, + allow_nan=False, sort_keys=True, separators=(",", ":")) + task["acceptance"].append(item) + if len(task["acceptance"]) != len(fields["acceptance"]): + raise ValueError("strategic acceptance criteria do not match the task") + used_strategic_bindings.add(ref) references[ref] = task_id tasks.append(task) by_id[task_id] = task @@ -338,6 +384,8 @@ def resolve(value: Any) -> str: ready.append(dependent) if resolved != len(tasks): raise ValueError("dependency cycle detected") + if used_strategic_bindings != set(strategic_bindings or {}): + raise ValueError("strategic bindings must identify added task references exactly") for task in tasks: task["plan_id"] = plan_id @@ -355,6 +403,16 @@ def resolve(value: Any) -> str: revision = self.store.apply_working_plan_edit( plan, tasks, expected_revision=expected_revision, expected_tasks=previous_tasks, user_id=self.user_id, workspace_id=self.workspace_id, session_id=self.session_id, + transaction_event=( + {"event_type": transaction_event["event_type"], **( + {"expected_proposal": transaction_event["expected_proposal"]} + if "expected_proposal" in transaction_event else {} + ), "payload": { + **transaction_event["payload"], "working_plan_id": plan_id, + "working_plan_revision": expected_revision + 1, + "working_task_ids": [task["id"] for task in tasks], + }} if transaction_event else None + ), ) working_plan = WorkingPlan( id=plan_id, complexity=complexity, status=plan_status, task_ids=tuple(task["id"] for task in tasks), @@ -582,7 +640,8 @@ def mutation_matches_current_task(self, action: str, args: str) -> tuple[bool, s def record_evidence(self, *, task_id: str | None = None, action: str, result: str, success: bool, acceptance: str | None = None, args: str | None = None, args_projection: str | None = None, - args_fingerprint: str | None = None, receipt_id: str | None = None) -> dict[str, Any]: + args_fingerprint: str | None = None, receipt_id: str | None = None, + verification_status: str | None = None) -> dict[str, Any]: canonical = _receipt_action(action) normalized_args = _receipt_args(args) persisted_args = _receipt_args(args_projection) if args_projection is not None else normalized_args @@ -624,6 +683,14 @@ def record_evidence(self, *, task_id: str | None = None, action: str, result: st persisted_targets = [] for task in targets: candidate = deepcopy(task) + if success and verification_status in {"verified", "not_applicable"}: + verified_ids = [criterion["id"] for criterion in candidate.get("acceptance", []) + if isinstance(criterion, dict) and criterion.get("id") + and criterion.get("action") == canonical + and criterion.get("args") == normalized_args] + if verified_ids: + item["acceptance_ids"] = verified_ids + item.setdefault("acceptance", "verified strategic criterion") if ordered_inferred: marker = _ordered_plan_info(task) if marker: diff --git a/tests/test_execution_receipts.py b/tests/test_execution_receipts.py index a2cadfb..744877d 100644 --- a/tests/test_execution_receipts.py +++ b/tests/test_execution_receipts.py @@ -299,6 +299,74 @@ def test_receipt_workspace_uses_project_task_scope(self): ) self.assertEqual(receipt.workspace_id, "project-workspace") + def test_strategic_criterion_coverage_requires_receipt_verification(self): + from dataclasses import replace + + runtime = self.app.runtime + arguments = {"path": "criterion.txt", "content": "verified"} + self.plan("write_file", arguments) + task = runtime.tasks.tasks[0] + task["acceptance"] = [{ + "id": "criterion-content", "criterion": "File has approved content", "action": "write_file", + "args": json.dumps(arguments, ensure_ascii=False, sort_keys=True, separators=(",", ":")), + }] + task.setdefault("checkpoint", {})["strategic_plan"] = {"acceptance_ids": ["criterion-content"]} + + for status in ("failed", "unavailable"): + receipt = runtime._make_execution_receipt( + action="write_file", args=arguments, authorized=True, started_at="start", success=True, + result="write reported", operation_scope="test", task_id=task["id"], + tool_result=ToolResult(text="write reported"), + ) + runtime._record_turn_receipt(replace( + receipt, verification={"status": status, "method": "fixture", "observed": None}, + )) + self.assertNotIn("acceptance_ids", task["evidence"][-1]) + + (self.root / "criterion.txt").write_text("verified", encoding="utf-8") + receipt = runtime._make_execution_receipt( + action="write_file", args=arguments, authorized=True, started_at="start", success=True, + result="write verified", operation_scope="test", task_id=task["id"], + tool_result=ToolResult(text="write verified"), + ) + self.assertEqual(receipt.verification["status"], "verified") + runtime._record_turn_receipt(receipt) + self.assertEqual(task["evidence"][-1]["acceptance_ids"], ["criterion-content"]) + + def test_strategic_task_stays_active_until_every_criterion_has_a_verified_receipt(self): + from dataclasses import replace + + runtime = self.app.runtime + write_args = {"path": "criterion.txt", "content": "verified"} + read_args = {"path": "criterion.txt"} + self.plan("write_file", write_args) + task = runtime.tasks.tasks[0] + criteria = [("criterion-content", "write_file", write_args), + ("criterion-readable", "read_file", read_args)] + task["acceptance"] = [{ + "id": criterion_id, "criterion": criterion_id, "action": action, + "args": json.dumps(arguments, ensure_ascii=False, sort_keys=True, separators=(",", ":")), + } for criterion_id, action, arguments in criteria] + task.setdefault("checkpoint", {})["strategic_plan"] = { + "acceptance_ids": [criterion_id for criterion_id, _, _ in criteria], + } + + for criterion_id, action, arguments in criteria: + if action == "write_file": + (self.root / "criterion.txt").write_text("verified", encoding="utf-8") + receipt = runtime._make_execution_receipt( + action=action, args=arguments, authorized=True, started_at="start", success=True, + result="verified", operation_scope="test", task_id=task["id"], + tool_result=ToolResult(text="verified"), + ) + runtime._record_turn_receipt(replace( + receipt, verification={"status": "verified", "method": "fixture", "observed": None}, + )) + if criterion_id == "criterion-content": + self.assertIn(task["status"], {"pending", "running"}) + self.assertIsNotNone(runtime.tasks.active_working_task(task["id"])) + self.assertEqual(task["status"], "succeeded") + def test_legacy_execute_persists_receipt_in_project_scope(self): runtime = self.app.runtime session = runtime.current_session diff --git a/tests/test_interaction.py b/tests/test_interaction.py index 501df07..8c0a019 100644 --- a/tests/test_interaction.py +++ b/tests/test_interaction.py @@ -2,6 +2,8 @@ import tempfile import unittest from openkyrozen.tools.models import ToolResult +from types import SimpleNamespace +from openkyrozen.agent.executor import _strategic_plan_denial from pathlib import Path from unittest.mock import patch @@ -14,14 +16,371 @@ is_plan_acceptance, mode_capabilities, parse_control_block, + render_plan, route_mode, split_inline_command, + validate_plan_proposal, ) from openkyrozen.tasks.engine import TaskManager from openkyrozen.security.capabilities import issue_capability_token from openkyrozen.workspace.context import resolve_launch_context +class StrategicPlanExecutionBoundsTests(unittest.TestCase): + def test_tool_effect_capability_risk_and_workspace_must_fit_accepted_bounds(self): + from pathlib import Path + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + plan = {"bounds": {"workspace_paths": ["src"], "capabilities": ["read", "write"], + "effects": ["read_only", "filesystem"], "risk_ceiling": "normal"}} + write = SimpleNamespace(capability="write", side_effects="filesystem", risk="normal") + self.assertIsNone(_strategic_plan_denial(plan, "write_file", {"path": "src/app.py"}, write, root)) + self.assertIn("outside", _strategic_plan_denial( + plan, "write_file", {"path": "README.md"}, write, root)) + shell = SimpleNamespace(capability="shell", side_effects="process", risk="normal") + self.assertIn("capability", _strategic_plan_denial( + plan, "run_cmd", {"command": "make test"}, shell, root)) + high = SimpleNamespace(capability="write", side_effects="filesystem", risk="high") + self.assertIn("risk", _strategic_plan_denial( + plan, "write_file", {"path": "src/app.py"}, high, root)) + + def test_legacy_bounds_fail_closed_for_unapproved_effects(self): + from pathlib import Path + with tempfile.TemporaryDirectory() as directory: + plan = {"bounds": {"workspace_paths": ["."], "capabilities": ["read"], + "effects": ["read_only"], "risk_ceiling": "normal"}} + write = SimpleNamespace(capability="write", side_effects="filesystem", risk="normal") + self.assertIn("capability", _strategic_plan_denial( + plan, "write_file", {"path": "file.txt"}, write, Path(directory))) + + def test_workspace_scope_resolves_symlinks_before_authorizing(self): + from pathlib import Path + with tempfile.TemporaryDirectory() as directory, tempfile.TemporaryDirectory() as outside: + root = Path(directory) + (root / "src").symlink_to(Path(outside), target_is_directory=True) + plan = {"bounds": {"workspace_paths": ["src"], "capabilities": ["write"], + "effects": ["filesystem"], "risk_ceiling": "normal"}} + write = SimpleNamespace(capability="write", side_effects="filesystem", risk="normal") + self.assertIn("outside", _strategic_plan_denial( + plan, "write_file", {"path": "src/payload.txt"}, write, root)) + + def test_opaque_shell_process_is_denied_under_path_scoped_plan(self): + from pathlib import Path + with tempfile.TemporaryDirectory() as directory: + plan = {"bounds": {"workspace_paths": ["src"], "capabilities": ["shell"], + "effects": ["process"], "risk_ceiling": "normal"}} + shell = SimpleNamespace(capability="shell", side_effects="process", risk="normal") + self.assertIn("cannot be confined", _strategic_plan_denial( + plan, "run_cmd", {"command": "rm -rf /tmp/unrelated"}, shell, Path(directory))) + + +class StrategicPlanContractTests(unittest.TestCase): + def test_strategic_plan_has_stable_objective_criteria_and_approved_bounds(self): + proposal = { + "title": "Change", "summary": "Make the requested change.", "assumptions": [], + "bounds": { + "workspace_paths": ["src", "docs"], "capabilities": ["read", "write"], + "effects": ["read_only", "filesystem"], "risk_ceiling": "normal", + }, + "steps": [{"id": "implement", "title": "Implement", "description": "Update the feature.", + "acceptance": ["Behavior is correct", "Tests pass"]}], + } + + first = validate_plan_proposal(proposal) + repeated = validate_plan_proposal(proposal) + revised = validate_plan_proposal({**proposal, "summary": "Deliver the requested change."}, previous=first) + + self.assertEqual(first["steps"][0]["id"], "implement") + self.assertEqual(first["steps"][0]["acceptance_ids"], repeated["steps"][0]["acceptance_ids"]) + self.assertEqual(first["steps"][0]["acceptance_ids"], revised["steps"][0]["acceptance_ids"]) + self.assertEqual(first["bounds"]["workspace_paths"], ["docs", "src"]) + self.assertEqual(first["bounds"]["risk_ceiling"], "normal") + self.assertEqual(len(first["fingerprint"]), 64) + self.assertNotEqual(first["fingerprint"], revised["fingerprint"]) + rendered = render_plan(first) + self.assertIn("src", rendered) + self.assertIn("filesystem", rendered) + self.assertIn("Risk ceiling: normal", rendered) + self.assertIn(first["steps"][0]["acceptance_ids"][0], rendered) + + def test_legacy_strategic_plan_gets_conservative_bounds_and_invalid_scope_is_rejected(self): + legacy = validate_plan_proposal({ + "title": "Read only", "summary": "Inspect safely.", "steps": [{ + "id": "inspect", "title": "Inspect", "description": "Read the project.", + "acceptance": ["Relevant files are identified"], + }], + }) + self.assertEqual(legacy["bounds"], { + "workspace_paths": ["."], "capabilities": ["read"], + "effects": ["read_only"], "risk_ceiling": "normal", + }) + for bounds in ( + {"workspace_paths": ["../outside"], "capabilities": ["read"], + "effects": ["read_only"], "risk_ceiling": "normal"}, + {"workspace_paths": ["."], "capabilities": ["full"], + "effects": ["read_only"], "risk_ceiling": "normal"}, + {"workspace_paths": ["."], "capabilities": ["read"], + "effects": ["unknown"], "risk_ceiling": "normal"}, + {"workspace_paths": ["."], "capabilities": ["read"], + "effects": ["read_only"], "risk_ceiling": "unbounded"}, + ): + with self.subTest(bounds=bounds), self.assertRaises(InteractionError): + validate_plan_proposal({ + "title": "Unsafe", "summary": "Reject invalid bounds.", "bounds": bounds, + "steps": [{"id": "inspect", "title": "Inspect", "description": "Read.", + "acceptance": ["Inspected"]}], + }) + + +class StrategicPlanAcceptanceTests(unittest.TestCase): + def setUp(self): + self.directory = tempfile.TemporaryDirectory() + self.addCleanup(self.directory.cleanup) + self.store = EventStore(Path(self.directory.name) / "state.sqlite3") + self.controller = InteractionController( + self.store, workspace_id="project", session_id="strategic-session", + ) + self.manager = TaskManager( + self.store, workspace_id="project", session_id="strategic-session", + ) + + def _proposal(self): + return self.controller.propose_plan({ + "title": "Implement the page", "summary": "Create and verify one page.", + "bounds": { + "workspace_paths": ["site"], "capabilities": ["read", "write"], + "effects": ["read_only", "filesystem"], "risk_ceiling": "normal", + }, + "steps": [{"id": "page", "title": "Create page", "description": "Write the page.", + "acceptance": ["Page exists", "Page content is correct"]}], + }) + + def test_acceptance_atomically_links_working_plan_tasks_and_criteria(self): + plan = self._proposal() + + accepted, created = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + + self.assertTrue(created) + self.assertEqual(accepted["fingerprint"], plan["fingerprint"]) + self.assertIsNotNone(self.manager.working_plan) + task = self.manager.tasks[0] + binding = task["checkpoint"]["strategic_plan"] + self.assertEqual(binding["plan_id"], plan["plan_id"]) + self.assertEqual(binding["version"], plan["version"]) + self.assertEqual(binding["objective_id"], "page") + self.assertEqual(binding["acceptance_ids"], plan["steps"][0]["acceptance_ids"]) + event = next(event for event in self.store.list_events( + "interaction.plan_accepted", workspace_id="project", session_id="strategic-session", + )) + self.assertEqual(event["payload"]["working_plan_id"], self.manager.working_plan.id) + self.assertEqual(event["payload"]["fingerprint"], plan["fingerprint"]) + + def test_acceptance_event_failure_rolls_back_plan_tasks_and_event(self): + plan = self._proposal() + json_encoder = self.store.database._json + + def fail_acceptance(payload): + if isinstance(payload, dict) and payload.get("fingerprint") == plan["fingerprint"]: + raise ValueError("injected acceptance event failure") + return json_encoder(payload) + + with patch.object(self.store.database, "_json", side_effect=fail_acceptance): + with self.assertRaisesRegex(ValueError, "injected"): + self.controller.accept_plan(self.manager, plan_id=plan["plan_id"], version=plan["version"]) + + self.assertEqual(self.manager.tasks, []) + self.assertIsNone(self.manager.working_plan) + self.assertIsNotNone(self.controller.state()["pending_plan"]) + self.assertIsNone(self.store.get_working_plan( + user_id="local", workspace_id="project", session_id="strategic-session", + )) + self.assertEqual(self.store.list_events( + "interaction.plan_accepted", workspace_id="project", session_id="strategic-session", + ), []) + + def test_reaccepting_executing_plan_rejects_an_explicit_stale_version(self): + plan = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + with self.assertRaises(InteractionError): + self.controller.accept_plan( + self.manager, plan_id=accepted["plan_id"], version=accepted["version"] - 1, + ) + + def test_revised_proposal_versions_from_the_accepted_strategic_plan(self): + plan = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + revised = self.controller.propose_plan({ + "title": "Implement the page", "summary": "Also verify the page output.", + "bounds": plan["bounds"], "steps": plan["steps"], + }) + self.assertEqual(revised["plan_id"], accepted["plan_id"]) + self.assertEqual(revised["version"], accepted["version"] + 1) + self.assertNotEqual(revised["fingerprint"], accepted["fingerprint"]) + + def test_staging_and_cancelling_revision_preserves_accepted_plan_bounds(self): + plan = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + revision = self.controller.propose_plan({ + "title": "Implement the page", "summary": "Revise the intended outcome.", + "bounds": plan["bounds"], "steps": plan["steps"], + }) + state = self.controller.state() + self.assertEqual(state["executing_plan"]["fingerprint"], accepted["fingerprint"]) + self.assertEqual(state["pending_plan"]["fingerprint"], revision["fingerprint"]) + self.controller.cancel_plan(revision["plan_id"], revision["version"]) + self.assertEqual(self.controller.state()["executing_plan"]["fingerprint"], accepted["fingerprint"]) + + def test_acceptance_rejects_proposal_superseded_before_transaction(self): + plan = self._proposal() + edit_plan = self.manager.edit_plan + + def supersede_then_edit(**kwargs): + self.controller.propose_plan({ + "title": "Implement the page", "summary": "A newer proposal.", + "bounds": plan["bounds"], "steps": plan["steps"], + }) + return edit_plan(**kwargs) + + with patch.object(self.manager, "edit_plan", side_effect=supersede_then_edit): + with self.assertRaisesRegex(ValueError, "Strategic Plan proposal changed"): + self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + self.assertEqual(self.manager.tasks, []) + self.assertIsNone(self.manager.working_plan) + self.assertEqual(self.store.list_events( + "interaction.plan_accepted", workspace_id="project", session_id="strategic-session", + ), []) + + def test_completion_requires_evidence_for_every_approved_acceptance_id(self): + plan = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + task = self.manager.tasks[0] + task["status"] = "succeeded" + task["evidence"] = [{"success": True, "acceptance": "one check", "acceptance_ids": [ + plan["steps"][0]["acceptance_ids"][0], + ]}] + with self.assertRaisesRegex(InteractionError, "criteria have not all been verified"): + self.controller.complete_plan(accepted, task_manager=self.manager) + + def test_completion_records_success_after_every_criterion_has_successful_evidence(self): + plan = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + task = self.manager.tasks[0] + task["status"] = "succeeded" + task["evidence"] = [{"success": True, "acceptance": "verified", + "acceptance_ids": plan["steps"][0]["acceptance_ids"]}] + self.controller.complete_plan(accepted, task_manager=self.manager) + event = self.store.list_events( + "interaction.plan_completed", workspace_id="project", session_id="strategic-session", + ) + self.assertEqual(event[0]["payload"]["status"], "completed") + + def test_real_receipts_cover_only_their_matching_strategic_criteria(self): + proposal = self.controller.propose_plan({ + "title": "Verified page", "summary": "Create and inspect a page.", + "bounds": {"workspace_paths": ["site"], "capabilities": ["read", "write"], + "effects": ["filesystem", "read_only"], "risk_ceiling": "normal"}, + "steps": [{"id": "page", "title": "Create page", "description": "Create and inspect the page.", + "acceptance": [ + {"criterion": "Page exists", "verification": { + "action": "write_file", "arguments": {"path": "site/index.html", "content": "ok"}}}, + {"criterion": "Page can be read back", "verification": { + "action": "read_file", "arguments": {"path": "site/index.html"}}}, + ]}], + }) + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=proposal["plan_id"], version=proposal["version"], + ) + task = self.manager.tasks[0] + wrong = self.manager.record_evidence(task_id=task["id"], action="write_file", + args='{"content":"wrong","path":"site/index.html"}', + result="file written", success=True, verification_status="verified") + self.assertNotIn("acceptance_ids", wrong) + for verification_status in ("failed", "unavailable", None): + unverified = self.manager.record_evidence( + task_id=task["id"], action="write_file", args='{"content":"ok","path":"site/index.html"}', + result="reported success", success=True, verification_status=verification_status, + ) + self.assertNotIn("acceptance_ids", unverified) + first = self.manager.record_evidence(task_id=task["id"], action="write_file", + args='{"content":"ok","path":"site/index.html"}', + result="file written", success=True, verification_status="verified") + self.assertEqual(first["acceptance_ids"], [task["acceptance"][0]["id"]]) + self.assertNotIn(task["acceptance"][1]["id"], first["acceptance_ids"]) + second = self.manager.record_evidence(task_id=task["id"], action="read_file", + args='{"path":"site/index.html"}', result="ok", success=True, + verification_status="not_applicable") + self.assertEqual(second["acceptance_ids"], [task["acceptance"][1]["id"]]) + self.manager.set_status(0, "succeeded") + self.controller.complete_plan(accepted, task_manager=self.manager) + + def test_runtime_receipts_keep_multi_criterion_task_active_until_all_are_verified(self): + from openkyrozen.agent.executor import _record_turn_receipt + from openkyrozen.agent.types import ExecutionReceipt + from types import SimpleNamespace + import hashlib + + large_content = "x" * 1500 + proposal = self.controller.propose_plan({ + "title": "Verified page", "summary": "Create and inspect a page.", + "bounds": {"workspace_paths": ["site"], "capabilities": ["read", "write"], + "effects": ["filesystem", "read_only"], "risk_ceiling": "normal"}, + "steps": [{"id": "page", "title": "Create page", "description": "Create and inspect the page.", + "acceptance": [ + {"criterion": "Page exists", "verification": {"action": "write_file", + "arguments": {"path": "site/index.html", "content": large_content}}}, + {"criterion": "Page can be read back", "verification": {"action": "read_file", + "arguments": {"path": "site/index.html"}}}, + ]}], + }) + self.controller.accept_plan(self.manager, plan_id=proposal["plan_id"], version=proposal["version"]) + task = self.manager.tasks[0] + runtime = SimpleNamespace(tasks=self.manager, _emit_stream_event=lambda event: None) + + for index, (action, arguments) in enumerate(( + ("write_file", {"path": "site/index.html", "content": large_content}), + ("read_file", {"path": "site/index.html"}), + )): + if action == "write_file": + (Path(self.directory.name) / "site").mkdir(exist_ok=True) + (Path(self.directory.name) / "site/index.html").write_text(large_content, encoding="utf-8") + # Rebuild through the same evidence path used by runtime receipt handling. + receipt = ExecutionReceipt( + receipt_id=f"receipt-{index}", operation_id=f"operation-{index}", action=action, + args=json.dumps(arguments, sort_keys=True, separators=(",", ":"))[:1000], + args_sha256=hashlib.sha256(json.dumps( + arguments, ensure_ascii=False, sort_keys=True, separators=(",", ":") + ).encode()).hexdigest(), + authorized=True, started_at="start", completed_at="end", success=True, result="ok", + call_id=f"call-{index}", + call_id_source="native", arguments=arguments, tool_result=ToolResult(text="ok"), + user_id="local", workspace_id="project", session_id="strategic-session", + plan_id=self.manager.working_plan.id, task_id=task["id"], side_effects="filesystem", + verification={"status": "verified", "method": "fixture", "observed": None}, + ) + _record_turn_receipt(runtime, receipt) + if index == 0: + self.assertIn(task["status"], {"pending", "running"}) + self.assertIsNotNone(self.manager.active_working_task(task["id"])) + covered = {criterion_id for item in task["evidence"] + for criterion_id in item.get("acceptance_ids", [])} + self.assertEqual(covered, {item["id"] for item in task["acceptance"]}) + self.assertEqual(self.manager.tasks[0]["status"], "succeeded") + + class InteractionTests(unittest.TestCase): def setUp(self): self.directory = tempfile.TemporaryDirectory() @@ -238,6 +597,7 @@ def test_plan_revision_acceptance_is_versioned_and_idempotent(self): self.assertFalse(created_again) self.assertEqual(accepted, duplicate) self.assertEqual(len(manager.tasks), 1) + self.assertEqual(manager.tasks[0]["acceptance"][0]["criterion"], "Relevant path and tests are identified") self.assertEqual(self.controller.state()["effective_mode"], "agent") @@ -690,7 +1050,9 @@ def test_accepted_plan_rejects_unrelated_mutation_before_execution(self): ) proposal = controller.propose_plan({ "title": "Create page", "summary": "Create only the accepted page.", - "assumptions": [], "steps": [{ + "assumptions": [], "bounds": {"workspace_paths": ["."], + "capabilities": ["read", "write"], "effects": ["read_only", "filesystem"], + "risk_ceiling": "normal"}, "steps": [{ "id": "page", "title": "Create index.html", "description": "Write index.html with the requested markup.", "acceptance": ["index.html exists"], From befb08cc9a2c32cfcabbbee756171d3afaf35637 Mon Sep 17 00:00:00 2001 From: Evan Gong Date: Sat, 17 Oct 2026 00:11:20 +0800 Subject: [PATCH 02/16] fix: address strategic plan review findings --- docs/index.md | 1 + openkyrozen/agent/executor.py | 81 +++++++++++++++++--- openkyrozen/agent/loop.py | 13 +++- openkyrozen/agent/modes.py | 23 ++++-- openkyrozen/agent/prompts.py | 2 +- tests/test_interaction.py | 139 ++++++++++++++++++++++++++++++++-- 6 files changed, 232 insertions(+), 27 deletions(-) diff --git a/docs/index.md b/docs/index.md index 633f632..0d015ac 100644 --- a/docs/index.md +++ b/docs/index.md @@ -38,6 +38,7 @@ This index covers the current product guides, developer references, and dated en - [V3 SQLite migration plan](superpowers/plans/2026-10-14-v3-issue-264.md) records ordered schema migrations, backups, persisted approval history, and verification for issue #264. - [V3 Working Plan contract](superpowers/plans/2026-10-14-v3-issue-241.md) records plan ownership, complexity policy, lifecycle, persistence, and verification for issue #241. - [V3 atomic Working Plan edits](superpowers/plans/2026-10-16-v3-issue-242.md) records revision-checked operations, atomic persistence, ordering, dependencies, and validation for issue #242. +- [V3 Strategic Plan contract](superpowers/plans/2026-10-16-v3-issue-245.md) records approval versions, execution bounds, task links, and acceptance evidence for issue #245. - [V3 plan index](superpowers/plans/index.md) links the issue-specific implementation plans and identifies their coverage. - [V3 execution receipt implementation plan](superpowers/plans/2026-10-14-v3-issue-236.md) records receipt identity, explicit task binding, structured effect verification, compatibility, and validation for issue #236. - [V3 runtime events implementation plan](superpowers/plans/2026-10-14-v3-issue-255.md) records the canonical event contract, correlation, interface adapters, and validation for issue #255. diff --git a/openkyrozen/agent/executor.py b/openkyrozen/agent/executor.py index e3791a6..ed1e598 100644 --- a/openkyrozen/agent/executor.py +++ b/openkyrozen/agent/executor.py @@ -18,6 +18,49 @@ from openkyrozen.tasks.models import _receipt_args +def _legacy_object_arguments(args, spec): + if not isinstance(args, str) or not spec or not getattr(spec, "legacy_fields", ()): + return args if isinstance(args, dict) else {} + fields = args.split("|") + if len(fields) != len(spec.legacy_fields): + return {} + candidate = dict(zip(spec.legacy_fields, fields)) + try: + return candidate if spec.legacy_arguments(candidate) == args else {} + except ValueError: + return {} + + +def _file_mutation_matches_objective(task, arguments): + path = arguments.get("path") if isinstance(arguments, dict) else None + if not isinstance(path, str) or not path.strip(): + return False + normalized = path.replace("\\", "/").strip("/").casefold() + basename = normalized.rsplit("/", 1)[-1] + description = " ".join(str(task.get(field, "")) for field in ("title", "description")).replace("\\", "/").casefold() + explicit_paths = {match.removeprefix("./") for match in re.findall( + r"(? tuple[s "title": value.get("title") or value.get("plan_name"), "summary": value.get("summary") or value.get("overview") or value.get("goal"), "assumptions": value.get("assumptions") or [], + **({"bounds": value["bounds"]} if "bounds" in value else {}), "steps": steps, } @@ -477,7 +478,8 @@ def state(self, user_input: str = "") -> dict[str, Any]: if plan and payload.get("plan_id") == plan.get("plan_id"): plan = None elif event_type == "interaction.plan_completed": - if executing and payload.get("plan_id") == executing.get("plan_id"): + if (executing and payload.get("plan_id") == executing.get("plan_id") + and payload.get("status") in {"completed", "cancelled"}): executing = None effective = route_mode( preference, user_input, pending_plan=plan is not None, executing_plan=executing is not None, @@ -630,16 +632,25 @@ def complete_plan(self, plan: dict[str, Any], *, status: str = "completed", task_manager: Any | None = None) -> None: if status not in {"completed", "failed", "cancelled"}: raise InteractionError("Strategic Plan terminal status is invalid") - if any(event["event_type"] == "interaction.plan_completed" - and event.get("payload", {}).get("plan_id") == plan.get("plan_id") - and event.get("payload", {}).get("version") == plan.get("version") - for event in self._history()): + prior_completions = [event.get("payload", {}).get("status") for event in self._history() + if event["event_type"] == "interaction.plan_completed" + and event.get("payload", {}).get("plan_id") == plan.get("plan_id") + and event.get("payload", {}).get("version") == plan.get("version")] + if any(previous in {"completed", "cancelled"} or previous == status + for previous in prior_completions): return - task_ids = plan.get("working_task_ids") + working = task_manager.working_plan if task_manager is not None else None + task_ids = (list(working.task_ids) if working and working.id == plan.get("working_plan_id") + else plan.get("working_task_ids")) if status == "completed": if task_manager is None or not isinstance(task_ids, list) or not task_ids: raise InteractionError("Strategic Plan completion requires its linked Working Plan evidence") tasks = {task.get("id"): task for task in task_manager.tasks} + objectives = {step["id"] for step in plan.get("steps", [])} + for task_id in task_ids: + binding = (tasks.get(task_id, {}).get("checkpoint") or {}).get("strategic_plan") or {} + if binding.get("plan_id") != plan.get("plan_id") or binding.get("objective_id") not in objectives: + raise InteractionError("Working Plan contains a task not linked to the accepted Strategic Plan") if any(tasks.get(task_id, {}).get("status") != "succeeded" or not any(isinstance(item, dict) and item.get("success") is True and item.get("acceptance") for item in tasks[task_id].get("evidence", [])) diff --git a/openkyrozen/agent/prompts.py b/openkyrozen/agent/prompts.py index ceb57fb..ecbceaa 100644 --- a/openkyrozen/agent/prompts.py +++ b/openkyrozen/agent/prompts.py @@ -197,7 +197,7 @@ def _system_prompt(self, tools_list: str, agent_config: dict[str, Any] | None = + ( "The final plan must use this standalone control:\n" "PlanProposal:\n```json\n" - '{"title":"Plan title","summary":"Outcome","assumptions":[],"bounds":{"workspace_paths":["src"],"capabilities":["read","write"],"effects":["read_only","filesystem"],"risk_ceiling":"normal"},"steps":[{"id":"objective-1","title":"Step","description":"Work to perform after acceptance","acceptance":["Observable result"]}]}\n' + '{"title":"Plan title","summary":"Outcome","assumptions":[],"bounds":{"workspace_paths":["src"],"capabilities":["read","write"],"effects":["read_only","filesystem"],"risk_ceiling":"normal"},"steps":[{"id":"objective-1","title":"Step","description":"Work to perform after acceptance","acceptance":[{"criterion":"Observable result","verification":{"action":"read_file","arguments":{"path":"src/result.txt"}}}]}]}\n' "```\nUse 1-10 stable objective IDs and state the smallest workspace paths, capability/effect categories, and normal or high risk ceiling. Output no task or execution controls.\n" if interaction_mode == "plan" else "" ) diff --git a/tests/test_interaction.py b/tests/test_interaction.py index 8c0a019..ef7973f 100644 --- a/tests/test_interaction.py +++ b/tests/test_interaction.py @@ -3,7 +3,7 @@ import unittest from openkyrozen.tools.models import ToolResult from types import SimpleNamespace -from openkyrozen.agent.executor import _strategic_plan_denial +from openkyrozen.agent.executor import _legacy_object_arguments, _strategic_plan_denial from pathlib import Path from unittest.mock import patch @@ -27,6 +27,48 @@ class StrategicPlanExecutionBoundsTests(unittest.TestCase): + def test_full_path_objectives_do_not_match_a_sibling_with_the_same_basename(self): + from openkyrozen.agent.executor import _file_mutation_matches_objective + objective = {"title": "Write src/public/index.html", "description": "Update the public page."} + self.assertFalse(_file_mutation_matches_objective(objective, {"path": "src/private/index.html"})) + self.assertTrue(_file_mutation_matches_objective(objective, {"path": "src/public/index.html"})) + scoped_directory = {"title": "Create index.html in src/public", "description": "", + "acceptance": [{"args": '{"path":"src/public/index.html"}'}]} + self.assertTrue(_file_mutation_matches_objective(scoped_directory, {"path": "src/public/index.html"})) + self.assertFalse(_file_mutation_matches_objective(scoped_directory, {"path": "src/private/index.html"})) + + def test_legacy_mutation_arguments_are_normalized_for_scope_checks(self): + from pathlib import Path + with tempfile.TemporaryDirectory() as directory: + plan = {"bounds": {"workspace_paths": ["site"], "capabilities": ["write"], + "effects": ["filesystem"], "risk_ceiling": "normal"}} + spec = SimpleNamespace(capability="write", side_effects="filesystem", risk="normal", + legacy_fields=("path", "content"), + legacy_arguments=lambda value: "|".join((value["path"], value["content"]))) + self.assertEqual(_legacy_object_arguments("site/index.html|ok", spec), + {"path": "site/index.html", "content": "ok"}) + normalized = json.dumps(_legacy_object_arguments("site/index.html|ok", spec), + ensure_ascii=False, sort_keys=True, separators=(",", ":")) + manager = TaskManager(EventStore(Path(directory) / "match.sqlite3"), workspace_id="w", session_id="s") + self.assertGreater(manager._receipt_match_score({"acceptance": [{ + "action": "write_file", "args": normalized, + }]}, "write_file", normalized), 0) + self.assertIsNone(_strategic_plan_denial( + plan, "write_file", "site/index.html|ok", spec, Path(directory))) + + def test_find_and_git_path_arrays_are_checked_against_plan_bounds(self): + from pathlib import Path + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + plan = {"bounds": {"workspace_paths": ["src"], "capabilities": ["read", "git"], + "effects": ["read_only", "filesystem"], "risk_ceiling": "normal"}} + find = SimpleNamespace(capability="read", side_effects="read", risk="normal") + add = SimpleNamespace(capability="git", side_effects="mutation", risk="normal") + self.assertIn("outside", _strategic_plan_denial( + plan, "find_files", "*.py|tests", find, root)) + self.assertIn("outside", _strategic_plan_denial( + plan, "git_add", {"paths": ["src/a.py", "outside/b.py"]}, add, root)) + def test_tool_effect_capability_risk_and_workspace_must_fit_accepted_bounds(self): from pathlib import Path with tempfile.TemporaryDirectory() as directory: @@ -155,6 +197,43 @@ def _proposal(self): "acceptance": ["Page exists", "Page content is correct"]}], }) + def test_failed_plan_keeps_its_accepted_execution_bounds(self): + plan = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + self.controller.complete_plan(accepted, status="failed", task_manager=self.manager) + self.assertEqual(self.controller.state()["executing_plan"]["fingerprint"], accepted["fingerprint"]) + + def test_failed_plan_can_complete_after_resumed_work_passes_evidence(self): + plan = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + self.controller.complete_plan(accepted, status="failed", task_manager=self.manager) + task = self.manager.tasks[0] + task["status"] = "succeeded" + task["evidence"] = [{"success": True, "acceptance": "verified", + "acceptance_ids": plan["steps"][0]["acceptance_ids"]}] + self.controller.complete_plan(accepted, task_manager=self.manager) + self.assertIsNone(self.controller.state()["executing_plan"]) + + def test_completion_checks_current_working_plan_membership(self): + plan = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + task = self.manager.tasks[0] + task["status"] = "succeeded" + task["evidence"] = [{"success": True, "acceptance": "verified", + "acceptance_ids": plan["steps"][0]["acceptance_ids"]}] + self.manager.edit_plan(complexity="complex", plan_id=self.manager.working_plan.id, + expected_revision=self.manager.working_plan.revision, + operations=[{"op": "add", "ref": "extra", "task": { + "description": "Unbound extra work", "acceptance": ["Done"]}}]) + with self.assertRaisesRegex(InteractionError, "linked to the accepted Strategic Plan"): + self.controller.complete_plan(accepted, task_manager=self.manager) + def test_acceptance_atomically_links_working_plan_tasks_and_criteria(self): plan = self._proposal() @@ -601,8 +680,9 @@ def test_plan_revision_acceptance_is_versioned_and_idempotent(self): self.assertEqual(manager.tasks[0]["acceptance"][0]["criterion"], "Relevant path and tests are identified") self.assertEqual(self.controller.state()["effective_mode"], "agent") - self.controller.complete_plan(accepted) - self.assertEqual(self.controller.state()["effective_mode"], "ask") + with self.assertRaisesRegex(InteractionError, "requires its linked Working Plan evidence"): + self.controller.complete_plan(accepted) + self.assertEqual(self.controller.state()["effective_mode"], "agent") completed_duplicate, created_after_completion = self.controller.accept_plan( manager, plan_id=second["plan_id"], version=second["version"], ) @@ -646,8 +726,8 @@ def test_provider_plan_text_cannot_duplicate_an_accepted_checklist(self): evidence = manager.record_evidence( action="write_file", args="index.html|ready", result="Wrote index.html", success=True, ) - self.assertEqual(evidence["task_id"], manager.tasks[0]["id"]) - self.assertEqual(manager.tasks[0]["status"], "succeeded") + self.assertNotIn("task_id", evidence) + self.assertEqual(manager.tasks[0]["status"], "pending") finally: self.agent.tasks = previous_tasks self.agent._interaction_controller = previous_controller @@ -671,12 +751,15 @@ def test_provider_shaped_plan_discards_executable_step_metadata(self): "plan_name": "Status page", "mode": "plan", "do_not_execute_until_approved": True, "overview": "Build after approval.", "assumptions": ["Python exists"], + "bounds": {"workspace_paths": ["site"], "capabilities": ["read", "write"], + "effects": ["read_only", "filesystem"], "risk_ceiling": "normal"}, "steps": [{"step": 1, "title": "Create page", "details": "Write index.html.", "acceptance_criteria": ["index.html exists"]}], }) parsed = self.agent._observe_model_response(shaped) self.assertEqual(parsed["plan_proposal"]["title"], "Status page") self.assertEqual(parsed["plan_proposal"]["steps"][0]["id"], "step-1") + self.assertEqual(parsed["plan_proposal"]["bounds"]["workspace_paths"], ["site"]) self.assertEqual(parsed["tool_calls"], []) executable = json.dumps({ @@ -689,7 +772,7 @@ def test_provider_shaped_plan_discards_executable_step_metadata(self): self.assertEqual(sanitized["plan_proposal"]["title"], "Safe proposal") self.assertEqual( set(sanitized["plan_proposal"]["steps"][0]), - {"id", "title", "description", "acceptance"}, + {"id", "title", "description", "acceptance", "acceptance_ids", "acceptance_verifications"}, ) self.assertEqual( sanitized["plan_proposal"]["steps"][0]["acceptance"], @@ -814,6 +897,46 @@ def test_plan_prompt_advertises_only_read_and_network_tools(self): for forbidden in ("write_file", "run_cmd", "TaskList", "TaskDone", "DefineTool", "git_commit"): self.assertNotIn(forbidden, prompt) self.assertIn("The first Plan-mode response must be exactly one listed read-only Action", prompt) + self.assertIn('"criterion":"Observable result"', prompt) + self.assertIn('"verification":{"action":"read_file"', prompt) + + def test_bounded_write_objective_allows_mutation_with_a_separate_read_verifier(self): + from openkyrozen.agent.types import ExecutionReceipt + previous = self.agent._interaction_controller + previous_tasks = self.agent.tasks + previous_root = self.agent._get_workspace_root() + manager = TaskManager(self.store, workspace_id="workspace", session_id="session") + controller = InteractionController(self.store, workspace_id="workspace", session_id="session") + proposal = controller.propose_plan({ + "title": "Write page", "summary": "Write the page.", "bounds": { + "workspace_paths": ["site"], "capabilities": ["write"], + "effects": ["filesystem", "read_only"], "risk_ceiling": "normal", + }, "steps": [{"id": "page", "title": "Write page", "description": "Write the page.", + "acceptance": [{"criterion": "Page can be read", "verification": { + "action": "read_file", "arguments": {"path": "site/index.html"}, + }}]}], + }) + controller.accept_plan(manager, plan_id=proposal["plan_id"], version=proposal["version"]) + self.agent.tasks = manager + self.agent._interaction_controller = controller + self.agent._set_workspace_root(Path(self.directory.name)) + receipt = ExecutionReceipt( + "receipt", "operation", "write_file", "site/index.html|ok", True, + "start", "end", True, "written", call_id="call", call_id_source="synthetic", + arguments={"path": "site/index.html", "content": "ok"}, tool_result=ToolResult(text="written"), + ) + try: + with patch.object(self.agent.tasks, "mutation_matches_current_task", return_value=(False, "wrong task")) as match, \ + patch.object(self.agent, "_run_tool", return_value=receipt): + result = self.agent._execute_turn_action( + "write_file", "site/index.html|ok", operation_scope="test", successful_operations=set(), + ) + self.assertTrue(result.success, result.result) + match.assert_not_called() + finally: + self.agent._interaction_controller = previous + self.agent.tasks = previous_tasks + self.agent._set_workspace_root(previous_root) def test_plan_requires_inspection_and_normalizes_structured_prose(self): class LearningStub: @@ -1055,7 +1178,9 @@ def test_accepted_plan_rejects_unrelated_mutation_before_execution(self): "risk_ceiling": "normal"}, "steps": [{ "id": "page", "title": "Create index.html", "description": "Write index.html with the requested markup.", - "acceptance": ["index.html exists"], + "acceptance": [{"criterion": "index.html exists", "verification": { + "action": "read_file", "arguments": {"path": "index.html"}, + }}], }], }) controller.accept_plan( From b810d1d99bc9750d58d2de8ca385cdcfb71c57e0 Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 17 Oct 2026 01:08:20 +0800 Subject: [PATCH 03/16] fix: address strategic plan review findings --- .../plans/2026-10-16-v3-issue-245.md | 23 +++ openkyrozen/agent/executor.py | 71 ++++--- openkyrozen/agent/loop.py | 2 + openkyrozen/agent/modes.py | 79 +++++++- openkyrozen/agent/ports.py | 1 + openkyrozen/persistence/events.py | 11 + openkyrozen/persistence/store.py | 10 + openkyrozen/persistence/tasks.py | 9 + openkyrozen/tasks/engine.py | 39 ++++ tests/test_execution_receipts.py | 37 +++- tests/test_interaction.py | 189 +++++++++++++++++- 11 files changed, 417 insertions(+), 54 deletions(-) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index 73d3c27..6a79e87 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -97,3 +97,26 @@ Implementation started from PR #284 head `0d49d4a488f6a9894b42900054cf06c665b980 - PASS: `make lint` and `git diff --check` after review fixes. - BLOCKED: `make check` passed Python architecture, syntax, inventory, and shell checks but the Go test exhausted the host's 117 MiB remaining disk. The Go cache was cleared; rerun is pending. - BLOCKED: `make docs-check` reaches the inventory generator but the installed Python 3.14 `pydantic_core` extension cannot load under Python 3.12. + +### Final-head review follow-up: PR #285 + +Before changing code, reproduce the eight outstanding findings on head `befb08c` and add regressions. Keep the scope within #245 and its Working Plan boundary; do not start #246 here. + +1. Parse legacy JSON-object arguments before Strategic Plan path/effect checks, so serialized `search_files` and `edit_file` paths cannot bypass accepted roots. +2. Canonicalize legacy arguments once for `ExecutionReceipt` and acceptance matching, without changing the string passed to legacy executors. +3. Reject non-string `risk_ceiling` values with `InteractionError` rather than leaking `TypeError` from set membership. +4. Require Strategic Plan objective/criterion bindings for Working Plan additions or semantic updates while a plan is executing; preserve valid bindings for allowed ordering and dependency edits. +5. Permit the exact staged material revision to replace its previous executing revision through one transaction, retaining prior evidence and preventing simultaneous active plans. +6. Reconcile completion from durable terminal Working Plan state after restart so a crash between task terminalization and completion-event append cannot strand Agent mode. +7. Make concurrent duplicate acceptance idempotent inside the transaction by returning the already-linked plan for the same accepted version/fingerprint. +8. Reconstruct active accepted bounds independently of the capped generic event listing so long sessions cannot lose capability, effect, risk, or workspace checks. + +All eight findings were reproduced and fixed; regressions cover each boundary. Also fixed two issues exposed while reproducing the findings: receipt evidence now preserves the Strategic Plan checkpoint, and normalized proposals retain their verification contracts on revalidation. + +- PASS: `tests.test_interaction tests.test_working_plans tests.test_execution_receipts` — 138 tests on Python 3.12 and 138 on Python 3.13. +- PASS: `make check`, `make lint`, and `make docs-check` with the supported Python 3.12 environment and writable temporary Go/Python caches. +- FAIL (pre-existing): full Python 3.12 suite — 735 tests, 2 failures, 146 errors. The strategic delegation test also fails on the original PR #285 head `befb08c`; one unrelated prompt-size test fails at 10,002 characters; most errors are SQLite access failures against protected host state. CI on the published head is still needed for full supported-runtime confirmation. +- BLOCKED: browser-backed acceptance and full Python 3.13 suite were not run locally. The focused Python 3.13 suites passed; browser and full-suite coverage remain delegated to CI. +- PASS: `git diff --check`; independent manual review found no additional actionable defects in the changed paths. + +The update remains limited to #245. Leave PR #285 and issue #245 open and unmerged. diff --git a/openkyrozen/agent/executor.py b/openkyrozen/agent/executor.py index ed1e598..5d5b267 100644 --- a/openkyrozen/agent/executor.py +++ b/openkyrozen/agent/executor.py @@ -15,7 +15,6 @@ render_tool_result, tool_failure) from openkyrozen.security.tool_policy import tool_capability from openkyrozen.agent.types import ExecutionReceipt -from openkyrozen.tasks.models import _receipt_args def _legacy_object_arguments(args, spec): @@ -82,7 +81,15 @@ def _strategic_plan_denial(plan, action, args, spec, workspace_root): if spec.risk == "high" and bounds.get("risk_ceiling") != "high": return "tool risk exceeds the accepted plan" - structured = args if isinstance(args, dict) else _legacy_object_arguments(args, spec) + structured = args if isinstance(args, dict) else None + if structured is None and isinstance(args, str) and args.lstrip().startswith("{"): + try: + parsed = json.loads(args) + except (json.JSONDecodeError, TypeError): + parsed = None + structured = parsed if isinstance(parsed, dict) else None + if structured is None: + structured = _legacy_object_arguments(args, spec) path_fields = {"path", "file_path", "directory", "destination", "target_path", "clone_path"} paths = {key: value for key, value in structured.items() if key in path_fields and isinstance(value, str) and value.strip()} @@ -171,6 +178,7 @@ def _make_execution_receipt(self, *, action: str, args: Any, authorized: bool, s tool_error: ToolError | None = None, result_data: Any = None, tool_result: ToolResult | None = None, call_id: str | None = None, call_id_source: str = "legacy", task_id: str | None = None, + receipt_arguments: dict[str, Any] | None = None, expected_spec=None) -> ExecutionReceipt: canonical = self._operation_action(action) from openkyrozen.agent.delegation import TOOLS @@ -183,8 +191,10 @@ def _make_execution_receipt(self, *, action: str, args: Any, authorized: bool, s tool_result = result result = render_tool_result(tool_result) success = tool_result.success if tool_result is not None else bool(success) - effect, effect_verified = self._subagent_tool_evidence(canonical, args, result_text, success=success) - acceptance, accepted = self._acceptance_for_tool(canonical, args, result_text, success=success) + canonical_arguments = receipt_arguments if receipt_arguments is not None else args if isinstance(args, dict) else None + evidence_arguments = canonical_arguments if canonical_arguments is not None else args + effect, effect_verified = self._subagent_tool_evidence(canonical, evidence_arguments, result_text, success=success) + acceptance, accepted = self._acceptance_for_tool(canonical, evidence_arguments, result_text, success=success) verification = {"status": "unavailable", "method": "none", "observed": None} if command is not None: verification = { @@ -234,13 +244,12 @@ def clean(item, remaining, depth=0): return ExecutionReceipt( receipt_id=f"receipt_{uuid.uuid4().hex}", - operation_id=self._operation_id(operation_scope, canonical, args), + operation_id=self._operation_id(operation_scope, canonical, + canonical_arguments if canonical_arguments is not None else args), action=canonical, args=self._fix_safe_text(self._operation_args(canonical, args), 1000), - args_sha256=hashlib.sha256( - (self._operation_args(canonical, args) if isinstance(args, dict) - else _receipt_args(self._operation_args(canonical, args))).encode("utf-8") - ).hexdigest(), + args_sha256=(hashlib.sha256(self._operation_args(canonical, canonical_arguments).encode("utf-8")).hexdigest() + if canonical_arguments is not None else None), authorized=authorized, started_at=started_at, completed_at=utc_now(), @@ -255,7 +264,7 @@ def clean(item, remaining, depth=0): result_data=result_data, call_id=call_id, call_id_source=call_id_source, - arguments=args if isinstance(args, dict) else None, + arguments=canonical_arguments, tool_result=tool_result, user_id=getattr(task_manager, "user_id", None) or getattr(getattr(self, "memory_bank", None), "user_id", None), workspace_id=getattr(task_manager, "workspace_id", None) or getattr(getattr(self, "memory_bank", None), "workspace_id", None), @@ -279,6 +288,19 @@ def _run_tool(self, action: str, args: str, *, return_success: bool = False, action = self.TOOL_ALIASES.get(action, action) receipt_args = _typed_arguments if _typed_arguments is not None else args approved_spec = _expected_spec or self.tool_registry.specs.get(action) + receipt_arguments = _typed_arguments if _typed_arguments is not None else args if isinstance(args, dict) else None + legacy_receipt_source = args + parsed_arguments = None + if isinstance(args, str) and args.lstrip().startswith("{"): + try: + parsed_arguments = json.loads(args) + except (json.JSONDecodeError, TypeError): + try: + parsed_arguments = ast.literal_eval(args) + except (ValueError, SyntaxError): + parsed_arguments = None + if isinstance(parsed_arguments, dict): + receipt_arguments = parsed_arguments if _typed_arguments is not None: args = _typed_arguments elif action in self.tool_registry.specs and self.tool_registry.get_spec(action).source == "dynamic": @@ -295,20 +317,20 @@ def _run_tool(self, action: str, args: str, *, return_success: bool = False, args = f"{args.get('file_path') or args.get('path') or ''}|{args.get('content') or args.get('text') or ''}" else: args = json.dumps(args) - elif isinstance(args, str) and args.startswith("{"): - try: - parsed = ast.literal_eval(args) - if isinstance(parsed, dict): - if action in ("run_cmd", "execute_terminal_command"): - args = parsed.get("cmd") or parsed.get("command") or "" - elif action == "write_file": - args = f"{parsed.get('file_path') or parsed.get('path') or ''}|{parsed.get('content') or parsed.get('text') or ''}" - else: - args = json.dumps(parsed) - except (ValueError, SyntaxError): - pass + elif isinstance(parsed_arguments, dict): + if action in ("run_cmd", "execute_terminal_command"): + args = parsed_arguments.get("cmd") or parsed_arguments.get("command") or "" + elif action == "write_file": + args = f"{parsed_arguments.get('file_path') or parsed_arguments.get('path') or ''}|{parsed_arguments.get('content') or parsed_arguments.get('text') or ''}" + else: + args = json.dumps(parsed_arguments) if isinstance(args, dict): - receipt_args = args + receipt_arguments = args + if not isinstance(receipt_args, dict): + receipt_args = args + elif receipt_arguments is None: + legacy_arguments = _legacy_object_arguments(legacy_receipt_source, approved_spec) + receipt_arguments = legacy_arguments or None self._publish_runtime_event( "tool.call", {"tool_name": action, "argument_keys": sorted(receipt_args)[:64] if isinstance(receipt_args, dict) else []}, @@ -318,7 +340,8 @@ def finish(outcome: ToolResult, authorized: bool = False, raw_result: Any = None text = render_tool_result(outcome) if return_receipt: return self._make_execution_receipt( - action=action, args=receipt_args, authorized=authorized, started_at=started_at, + action=action, args=receipt_args, receipt_arguments=receipt_arguments, + authorized=authorized, started_at=started_at, success=outcome.success, result=text, operation_scope=operation_scope, failure=outcome.error.code if outcome.error else None, raw_result=raw_result, tool_error=outcome.error, result_data=outcome.data, tool_result=outcome, diff --git a/openkyrozen/agent/loop.py b/openkyrozen/agent/loop.py index 6688721..3c2fbf5 100644 --- a/openkyrozen/agent/loop.py +++ b/openkyrozen/agent/loop.py @@ -28,6 +28,7 @@ def _chat_turn(self, user_input: str, clear_tasks: bool = False, profile: str | previous_capability_token = self._execution_capability_token original_user_input = user_input history_token: TurnToken | None = None + self._interaction_controller.reconcile_plan_completion(self.tasks) try: history_token = self.history_manager().begin_turn( conversation=list(self.short_term_memory), interaction=self._interaction_controller.state(user_input), @@ -107,6 +108,7 @@ def _chat_turn(self, user_input: str, clear_tasks: bool = False, profile: str | runtime.turn_end(reply="", success=False, error=f"{type(exc).__name__}: {exc}", **context) raise + self._interaction_controller.reconcile_plan_completion(self.tasks) state_after = self._interaction_controller.state() working_plan = self.tasks.working_plan active_task_ids = (list(working_plan.task_ids) diff --git a/openkyrozen/agent/modes.py b/openkyrozen/agent/modes.py index 882eb52..df9fb82 100644 --- a/openkyrozen/agent/modes.py +++ b/openkyrozen/agent/modes.py @@ -189,12 +189,17 @@ def validate_plan_proposal(value: Any, *, previous: dict[str, Any] | None = None acceptance = [acceptance] if not isinstance(acceptance, list) or not 1 <= len(acceptance) <= 5: raise InteractionError("each plan step requires 1-5 acceptance criteria") + supplied_verifications = step.get("acceptance_verifications") + if supplied_verifications is not None and ( + not isinstance(supplied_verifications, list) or len(supplied_verifications) != len(acceptance)): + raise InteractionError("acceptance verifications must align with acceptance criteria") acceptance_verifications = [] normalized_acceptance = [] - for item in acceptance: - verification = None + for criterion_index, item in enumerate(acceptance): + verification = supplied_verifications[criterion_index] if supplied_verifications is not None else None if isinstance(item, dict) and set(item) == {"criterion", "verification"}: verification = item["verification"] + if verification is not None: if (not isinstance(verification, dict) or set(verification) != {"action", "arguments"} or not isinstance(verification.get("action"), str) or not verification["action"].strip() or not isinstance(verification.get("arguments"), dict)): @@ -278,7 +283,7 @@ def values(field: str, allowed: set[str] | None = None) -> list[str]: "read_only", "filesystem", "process", "remote", "destructive", "internal", }) risk_ceiling = raw_bounds.get("risk_ceiling", "normal") - if risk_ceiling not in {"normal", "high"}: + if not isinstance(risk_ceiling, str) or risk_ceiling not in {"normal", "high"}: raise InteractionError("risk ceiling must be normal or high") bounds = { "workspace_paths": sorted(normalised_paths), "capabilities": sorted(capabilities), @@ -432,8 +437,12 @@ def _append(self, event_type: str, payload: dict[str, Any]) -> None: ) def _history(self) -> list[dict[str, Any]]: + if callable(getattr(self.store, "list_interaction_events", None)): + return self.store.list_interaction_events( + workspace_id=self.workspace_id, session_id=self.session_id, user_id=self.user_id, + ) events = self.store.list_events( - limit=10_000, workspace_id=self.workspace_id, session_id=self.session_id, + limit=None, workspace_id=self.workspace_id, session_id=self.session_id, user_id=self.user_id, ) return [event for event in reversed(events) @@ -593,18 +602,35 @@ def accept_plan(self, task_manager: Any, *, plan_id: str | None = None, if version is not None and version != executing.get("version"): raise InteractionError("accepted plan version is stale") return executing, False - if plan and executing: - raise InteractionError("a revised Strategic Plan requires reapproval before replacing the active plan") if not plan or (plan_id and plan_id != plan.get("plan_id")) or (version is not None and version != plan.get("version")): accepted = self.accepted_plan(plan_id, version) if accepted: return accepted, False raise InteractionError("plan is not pending") + if executing and task_manager.working_plan is None: + task_manager.recover() + working_plan = task_manager.working_plan if executing else None + previous_tasks = [task for task in task_manager.tasks + if working_plan and task.get("id") in working_plan.task_ids] + if any(task.get("status") == "running" for task in previous_tasks): + raise InteractionError("cannot replace a Strategic Plan while a Working Plan task is running") operations = [] strategic_bindings = {} + reused: set[str] = set() for step in plan["steps"]: + description = f"{step['title']}: {step['description']}" + existing = next((task for task in previous_tasks + if task.get("status") == "succeeded" + and (task.get("checkpoint") or {}).get("strategic_plan", {}).get("objective_id") == step["id"] + and (task.get("checkpoint") or {}).get("strategic_plan", {}).get("acceptance_ids") == step["acceptance_ids"] + and task.get("description") == description + and [item.get("criterion") if isinstance(item, dict) else item + for item in task.get("acceptance", [])] == step["acceptance"]), None) + if existing: + reused.add(existing["id"]) + continue operations.append({"op": "add", "ref": step["id"], "task": { - "description": f"{step['title']}: {step['description']}", + "description": description, "acceptance": step["acceptance"], }}) strategic_bindings[step["id"]] = { @@ -613,8 +639,17 @@ def accept_plan(self, task_manager: Any, *, plan_id: str | None = None, "acceptance_ids": step["acceptance_ids"], "acceptance_verifications": step["acceptance_verifications"], } - working = task_manager.edit_plan( - complexity="complex", expected_revision=0, operations=operations, + for task in previous_tasks: + if task["id"] not in reused and task.get("status") in {"pending", "failed", "blocked"}: + operations.insert(0, {"op": "cancel", "task": task["id"]}) + if working_plan and not operations: + task = previous_tasks[0] + operations.append({"op": "update", "task": task["id"], "fields": {"priority": task["priority"]}}) + try: + working = task_manager.edit_plan( + complexity=working_plan.complexity if working_plan else "complex", + plan_id=working_plan.id if working_plan else None, + expected_revision=working_plan.revision if working_plan else 0, operations=operations, strategic_bindings=strategic_bindings, transaction_event={"event_type": "interaction.plan_accepted", "payload": { "plan_id": plan["plan_id"], "version": plan["version"], @@ -623,10 +658,20 @@ def accept_plan(self, task_manager: Any, *, plan_id: str | None = None, "plan_id": plan["plan_id"], "version": plan["version"], "fingerprint": plan["fingerprint"], }}, - ) + ) + except ValueError: + accepted = self.accepted_plan(plan["plan_id"], plan["version"]) + if accepted and accepted.get("fingerprint") == plan["fingerprint"]: + return accepted, False + raise + bound_ids = [task["id"] for task in working["tasks"] + if task.get("status") != "cancelled" + and (task.get("checkpoint") or {}).get("strategic_plan", {}).get("plan_id") == plan["plan_id"] + and (task.get("checkpoint") or {}).get("strategic_plan", {}).get("objective_id") + in {step["id"] for step in plan["steps"]}] return {**plan, "working_plan_id": working["plan_id"], "working_plan_revision": working["revision"], - "working_task_ids": [task["id"] for task in working["tasks"]]}, True + "working_task_ids": bound_ids}, True def complete_plan(self, plan: dict[str, Any], *, status: str = "completed", task_manager: Any | None = None) -> None: @@ -675,6 +720,18 @@ def complete_plan(self, plan: dict[str, Any], *, status: str = "completed", "working_task_ids": task_ids or [], "status": status, }) + def reconcile_plan_completion(self, task_manager: Any) -> None: + plan = self.state().get("executing_plan") + if not plan or not plan.get("working_plan_id"): + return + working = task_manager.working_plan + if working is None or working.id != plan["working_plan_id"]: + working = task_manager.restore_working_plan(plan["working_plan_id"]) + if working is not None and working.status in {"completed", "cancelled"}: + self.complete_plan( + plan, status=working.status, task_manager=task_manager, + ) + def render_question(question: dict[str, Any]) -> str: lines = ["I need a little more detail before continuing:"] diff --git a/openkyrozen/agent/ports.py b/openkyrozen/agent/ports.py index 63892f7..663a4ef 100644 --- a/openkyrozen/agent/ports.py +++ b/openkyrozen/agent/ports.py @@ -9,3 +9,4 @@ class InteractionStore(Protocol): def append_event(self, event_type: str, payload: Any, **scope) -> str: ... def list_events(self, event_type: str | None = None, **scope) -> list[dict]: ... + def list_interaction_events(self, **scope) -> list[dict]: ... diff --git a/openkyrozen/persistence/events.py b/openkyrozen/persistence/events.py index b8f4030..1bb971c 100644 --- a/openkyrozen/persistence/events.py +++ b/openkyrozen/persistence/events.py @@ -45,6 +45,17 @@ def list_events(self, event_type: str | None = None, *, limit: int | None = 100, ).fetchall() return [dict(row, payload=self.database._loads(row["payload"], {})) for row in rows] + def list_interaction_events(self, *, workspace_id: str, session_id: str, + user_id: str) -> list[dict[str, Any]]: + with self.database.connection() as db: + rows = db.execute( + "SELECT * FROM events WHERE workspace_id=? AND session_id=? AND user_id=? " + "AND (event_type LIKE 'interaction.%' OR event_type='history.restored') " + "ORDER BY created_at,rowid", + (workspace_id, session_id, user_id), + ).fetchall() + return [dict(row, payload=self.database._loads(row["payload"], {})) for row in rows] + def list_sessions(self, *, workspace_id: str = "default", limit: int = 100, user_id: str | None = None) -> list[dict[str, Any]]: diff --git a/openkyrozen/persistence/store.py b/openkyrozen/persistence/store.py index 745f120..6d0fe6e 100644 --- a/openkyrozen/persistence/store.py +++ b/openkyrozen/persistence/store.py @@ -44,6 +44,11 @@ def delete_workspace_data(self, *, user_id: str, workspace_id: str, registry_wor def list_events(self, event_type: str | None=None, *, limit: int=100, workspace_id: str='default', session_id: str | None=None, user_id: str | None=None): return self.events.list_events(event_type, limit=limit, workspace_id=workspace_id, session_id=session_id, user_id=user_id) + def list_interaction_events(self, *, workspace_id: str, session_id: str, user_id: str): + return self.events.list_interaction_events( + workspace_id=workspace_id, session_id=session_id, user_id=user_id, + ) + def list_sessions(self, *, workspace_id: str='default', limit: int=100, user_id: str | None=None): return self.events.list_sessions(workspace_id=workspace_id, limit=limit, user_id=user_id) @@ -162,6 +167,11 @@ def write_task(self, task, *, user_id, workspace_id, session_id, expected_status def get_working_plan(self, *, user_id, workspace_id, session_id): return self.tasks.get_working_plan(user_id=user_id, workspace_id=workspace_id, session_id=session_id) + def get_working_plan_by_id(self, plan_id, *, user_id, workspace_id, session_id): + return self.tasks.get_working_plan_by_id( + plan_id, user_id=user_id, workspace_id=workspace_id, session_id=session_id, + ) + def set_working_plan_status(self, plan_id, status, *, user_id, workspace_id, session_id, **expected): return self.tasks.set_working_plan_status( plan_id, status, user_id=user_id, workspace_id=workspace_id, session_id=session_id, **expected, diff --git a/openkyrozen/persistence/tasks.py b/openkyrozen/persistence/tasks.py index 9472202..60fcca4 100644 --- a/openkyrozen/persistence/tasks.py +++ b/openkyrozen/persistence/tasks.py @@ -79,6 +79,15 @@ def get_working_plan(self, *, user_id: str, workspace_id: str, session_id: str) ).fetchone() return dict(row) if row is not None else None + def get_working_plan_by_id(self, plan_id: str, *, user_id: str, + workspace_id: str, session_id: str) -> dict[str, Any] | None: + with self.database.connection() as db: + row = db.execute( + "SELECT * FROM working_plans WHERE id=? AND user_id=? AND workspace_id=? AND session_id=?", + (plan_id, user_id, workspace_id, session_id or ""), + ).fetchone() + return dict(row) if row is not None else None + def set_working_plan_status(self, plan_id: str, status: str, *, user_id: str, workspace_id: str, session_id: str | None, expected_revision: int | None = None, diff --git a/openkyrozen/tasks/engine.py b/openkyrozen/tasks/engine.py index 3fa78c0..d4931d7 100644 --- a/openkyrozen/tasks/engine.py +++ b/openkyrozen/tasks/engine.py @@ -231,6 +231,14 @@ def edit_plan(self, *, expected_revision: int, operations: list[dict[str, Any]], original_ids = {task["id"] for task in previous_tasks} tasks = deepcopy(previous_tasks) by_id = {task["id"]: task for task in tasks} + has_strategic_tasks = any( + isinstance(task.get("checkpoint"), dict) and isinstance(task["checkpoint"].get("strategic_plan"), dict) + for task in previous_tasks + ) + is_strategic_acceptance = bool( + transaction_event and transaction_event.get("event_type") == "interaction.plan_accepted" + and transaction_event.get("expected_proposal") + ) references: dict[str, str] = {} used_strategic_bindings: set[str] = set() now = utc_now() @@ -263,6 +271,8 @@ def resolve(value: Any) -> str: if not isinstance(ref, str) or not ref.strip() or ref in references or ref in by_id: raise ValueError("task references must be unique nonempty strings") fields = self._plan_task_fields(operation["task"]) + if has_strategic_tasks and (not is_strategic_acceptance or not strategic_bindings or ref not in strategic_bindings): + raise ValueError("new work in a Strategic Plan requires an approved strategic binding") task_id = f"task_{scope_key}_{uuid.uuid4().hex}" task = {"id": task_id, **fields, "status": "pending", "attempts": 0, "checkpoint": fields["checkpoint"], "evidence": [], "created_at": now, @@ -276,6 +286,11 @@ def resolve(value: Any) -> str: or not isinstance(binding.get("acceptance_ids"), list) or not isinstance(binding.get("acceptance_verifications"), list)): raise ValueError("strategic task binding is invalid") + accepted_payload = (transaction_event or {}).get("payload", {}) + if (not is_strategic_acceptance + or any(binding.get(key) != accepted_payload.get(key) + for key in ("plan_id", "version", "fingerprint"))): + raise ValueError("strategic task binding must match the approved plan") task["checkpoint"] = {**task["checkpoint"], "strategic_plan": deepcopy(binding)} verifications = binding["acceptance_verifications"] if len(verifications) != len(fields["acceptance"]): @@ -305,6 +320,10 @@ def resolve(value: Any) -> str: if not isinstance(fields, dict) or not fields or set(fields) - {"description", "acceptance", "dependencies", "priority", "execution"}: raise ValueError("update fields are empty or unsupported") current = by_id[task_id] + if (has_strategic_tasks and isinstance(current.get("checkpoint"), dict) + and isinstance(current["checkpoint"].get("strategic_plan"), dict) + and set(fields) - {"priority", "dependencies"}): + raise ValueError("Strategic Plan task intent cannot change without approval") validated = self._plan_task_fields({ "description": fields.get("description", current["description"]), "acceptance": fields.get("acceptance", []), @@ -702,6 +721,7 @@ def record_evidence(self, *, task_id: str | None = None, action: str, result: st } elif inferred and not (task.get("checkpoint") or {}).get("action"): candidate["checkpoint"] = { + **(task.get("checkpoint") or {}), "action": canonical, "args": persisted_args, **({"args_sha256": args_fingerprint} if args_fingerprint else {}), } @@ -1031,4 +1051,23 @@ def recover(self) -> list[dict[str, Any]]: self.tasks = unfinished return unfinished + def restore_working_plan(self, plan_id: str) -> WorkingPlan | None: + saved = self.store.get_working_plan_by_id( + plan_id, user_id=self.user_id, workspace_id=self.workspace_id, + session_id=self.session_id or "", + ) + if saved is None: + return None + tasks = self.store.list_working_plan_tasks( + plan_id, user_id=self.user_id, workspace_id=self.workspace_id, + session_id=self.session_id or "", + ) + self.working_plan = WorkingPlan( + id=saved["id"], complexity=saved["complexity"], status=saved["status"], + task_ids=tuple(task["id"] for task in tasks), created_at=saved["created_at"], + updated_at=saved["updated_at"], revision=int(saved.get("revision", 0)), + ) + self.tasks = tasks + [task for task in self.tasks if task["id"] not in self.working_plan.task_ids] + return self.working_plan + from .worker import TaskWorker diff --git a/tests/test_execution_receipts.py b/tests/test_execution_receipts.py index 744877d..f0299f6 100644 --- a/tests/test_execution_receipts.py +++ b/tests/test_execution_receipts.py @@ -264,6 +264,16 @@ def test_native_call_does_not_infer_task_association(self): self.assertEqual(task["evidence"], []) self.assertEqual(task["status"], "pending") + def test_legacy_tool_receipt_uses_canonical_arguments_for_matching(self): + receipt = self.app.runtime._run_tool( + "write_file", "notes.txt|legacy pipe content", return_receipt=True, + operation_scope="review-regression", + ) + self.assertEqual(receipt.arguments, {"path": "notes.txt", "content": "legacy pipe content"}) + self.assertEqual(receipt.args_sha256, __import__("hashlib").sha256( + json.dumps(receipt.arguments, ensure_ascii=False, sort_keys=True, separators=(",", ":")).encode() + ).hexdigest()) + def test_early_legacy_rejection_publishes_correlated_tool_call(self): from openkyrozen.agent.events import RuntimeEventPublisher @@ -300,7 +310,11 @@ def test_receipt_workspace_uses_project_task_scope(self): self.assertEqual(receipt.workspace_id, "project-workspace") def test_strategic_criterion_coverage_requires_receipt_verification(self): - from dataclasses import replace + def verified(receipt, status): + return ExecutionReceipt.from_dict({ + **receipt.as_dict(), + "verification": {"status": status, "method": "fixture", "observed": None}, + }) runtime = self.app.runtime arguments = {"path": "criterion.txt", "content": "verified"} @@ -318,9 +332,7 @@ def test_strategic_criterion_coverage_requires_receipt_verification(self): result="write reported", operation_scope="test", task_id=task["id"], tool_result=ToolResult(text="write reported"), ) - runtime._record_turn_receipt(replace( - receipt, verification={"status": status, "method": "fixture", "observed": None}, - )) + runtime._record_turn_receipt(verified(receipt, status)) self.assertNotIn("acceptance_ids", task["evidence"][-1]) (self.root / "criterion.txt").write_text("verified", encoding="utf-8") @@ -334,12 +346,19 @@ def test_strategic_criterion_coverage_requires_receipt_verification(self): self.assertEqual(task["evidence"][-1]["acceptance_ids"], ["criterion-content"]) def test_strategic_task_stays_active_until_every_criterion_has_a_verified_receipt(self): - from dataclasses import replace + def verified(receipt): + return ExecutionReceipt.from_dict({ + **receipt.as_dict(), + "verification": {"status": "verified", "method": "fixture", "observed": None}, + }) runtime = self.app.runtime write_args = {"path": "criterion.txt", "content": "verified"} read_args = {"path": "criterion.txt"} - self.plan("write_file", write_args) + runtime.tasks.update_plan(complexity="complex", tasks=[{ + "description": "Verify requested content", + "acceptance": ["all declared criteria pass"], + }]) task = runtime.tasks.tasks[0] criteria = [("criterion-content", "write_file", write_args), ("criterion-readable", "read_file", read_args)] @@ -359,13 +378,11 @@ def test_strategic_task_stays_active_until_every_criterion_has_a_verified_receip result="verified", operation_scope="test", task_id=task["id"], tool_result=ToolResult(text="verified"), ) - runtime._record_turn_receipt(replace( - receipt, verification={"status": "verified", "method": "fixture", "observed": None}, - )) + runtime._record_turn_receipt(verified(receipt)) if criterion_id == "criterion-content": self.assertIn(task["status"], {"pending", "running"}) self.assertIsNotNone(runtime.tasks.active_working_task(task["id"])) - self.assertEqual(task["status"], "succeeded") + self.assertEqual(runtime.tasks.tasks[0]["status"], "succeeded") def test_legacy_execute_persists_receipt_in_project_scope(self): runtime = self.app.runtime diff --git a/tests/test_interaction.py b/tests/test_interaction.py index ef7973f..718c6c2 100644 --- a/tests/test_interaction.py +++ b/tests/test_interaction.py @@ -27,6 +27,30 @@ class StrategicPlanExecutionBoundsTests(unittest.TestCase): + def test_json_string_arguments_are_checked_against_accepted_paths(self): + with tempfile.TemporaryDirectory() as directory: + plan = {"bounds": {"workspace_paths": ["src"], "capabilities": ["read", "write"], + "effects": ["read_only", "filesystem"], "risk_ceiling": "normal"}} + read = SimpleNamespace(capability="read", side_effects="read_only", risk="normal") + write = SimpleNamespace(capability="write", side_effects="filesystem", risk="normal") + search_denial = _strategic_plan_denial( + plan, "search_files", '{"directory":"outside"}', read, Path(directory)) + self.assertIsNotNone(search_denial) + self.assertIn("outside", _strategic_plan_denial( + plan, "edit_file", '{"path":"outside/file.py","old_text":"x","new_text":"y"}', + write, Path(directory))) + + def test_non_string_risk_ceiling_is_a_plan_validation_error(self): + proposal = { + "title": "Change", "summary": "Make the change.", "steps": [{ + "title": "Update", "description": "Update safely.", "acceptance": ["Tests pass"], + }], + "bounds": {"workspace_paths": ["."], "capabilities": ["read"], + "effects": ["read_only"], "risk_ceiling": []}, + } + with self.assertRaises(InteractionError): + validate_plan_proposal(proposal) + def test_full_path_objectives_do_not_match_a_sibling_with_the_same_basename(self): from openkyrozen.agent.executor import _file_mutation_matches_objective objective = {"title": "Write src/public/index.html", "description": "Update the public page."} @@ -117,6 +141,20 @@ def test_opaque_shell_process_is_denied_under_path_scoped_plan(self): class StrategicPlanContractTests(unittest.TestCase): + def test_revalidating_a_normalized_plan_preserves_verified_acceptance_contracts(self): + proposal = { + "title": "Change", "summary": "Make the change.", + "steps": [{"id": "inspect", "title": "Inspect", "description": "Read the file.", + "acceptance": [{"criterion": "File is readable", "verification": { + "action": "read_file", "arguments": {"path": "src/file.txt"}, + }}]}], + } + first = validate_plan_proposal(proposal) + second = validate_plan_proposal(first) + self.assertEqual(second["steps"][0]["acceptance_verifications"], + first["steps"][0]["acceptance_verifications"]) + self.assertEqual(second["steps"][0]["acceptance_ids"], first["steps"][0]["acceptance_ids"]) + def test_strategic_plan_has_stable_objective_criteria_and_approved_bounds(self): proposal = { "title": "Change", "summary": "Make the requested change.", "assumptions": [], @@ -197,6 +235,131 @@ def _proposal(self): "acceptance": ["Page exists", "Page content is correct"]}], }) + def test_working_plan_edits_cannot_add_or_redefine_unbound_strategic_work(self): + plan = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + task_id = accepted["working_task_ids"][0] + with self.assertRaisesRegex(ValueError, "(?i)strategic"): + self.manager.edit_plan( + plan_id=self.manager.working_plan.id, expected_revision=self.manager.working_plan.revision, + operations=[{"op": "add", "ref": "unbound", "task": { + "description": "Write src/other.py", "acceptance": ["Other file is written"], + }}], + ) + with self.assertRaisesRegex(ValueError, "(?i)strategic"): + self.manager.edit_plan( + plan_id=self.manager.working_plan.id, expected_revision=self.manager.working_plan.revision, + operations=[{"op": "update", "task": task_id, "fields": { + "description": "Replace the objective with another file write", + }}], + ) + result = self.manager.edit_plan( + plan_id=self.manager.working_plan.id, expected_revision=self.manager.working_plan.revision, + operations=[{"op": "update", "task": task_id, "fields": {"priority": 1}}], + ) + binding = next(task for task in result["tasks"] if task["id"] == task_id)["checkpoint"]["strategic_plan"] + self.assertEqual(binding["fingerprint"], accepted["fingerprint"]) + + def test_accepting_material_revision_replaces_executing_plan_atomically(self): + first = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=first["plan_id"], version=first["version"], + ) + old_task_id = accepted["working_task_ids"][0] + revision = self.controller.propose_plan({ + "title": "Revised page", "summary": "Update the accepted page objective.", + "bounds": first["bounds"], + "steps": [{"id": "page", "title": "Create revised page", "description": "Write the revised page.", + "acceptance": ["Page exists", "Page content is correct"]}], + }) + updated, created = self.controller.accept_plan( + self.manager, plan_id=revision["plan_id"], version=revision["version"], + ) + self.assertTrue(created) + self.assertEqual(updated["version"], 2) + self.assertEqual(self.controller.state()["executing_plan"]["version"], 2) + self.assertIsNone(self.controller.state()["pending_plan"]) + self.assertEqual(self.manager.working_plan.id, accepted["working_plan_id"]) + self.assertEqual(next(task for task in self.manager.tasks if task["id"] == old_task_id)["status"], "cancelled") + self.assertEqual(len(updated["working_task_ids"]), 1) + self.assertNotEqual(updated["working_task_ids"][0], old_task_id) + + def test_restart_reconciles_completion_after_working_plan_terminalization(self): + plan = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + task = self.manager.tasks[0] + evidence = {"success": True, "acceptance": "verified", + "acceptance_ids": task["checkpoint"]["strategic_plan"]["acceptance_ids"]} + self.assertTrue(self.manager.set_status(0, "succeeded", evidence=evidence)) + self.assertEqual(self.manager.working_plan.status, "completed") + + restored_manager = TaskManager(self.store, workspace_id="project", session_id="strategic-session") + restored_manager.recover() + restored_controller = InteractionController( + self.store, workspace_id="project", session_id="strategic-session", + ) + restored_controller.reconcile_plan_completion(restored_manager) + self.assertIsNone(restored_controller.state()["executing_plan"]) + self.assertEqual(self.store.list_events( + "interaction.plan_completed", workspace_id="project", session_id="strategic-session", + )[-1]["payload"]["status"], "completed") + + def test_legacy_receipt_matching_preserves_strategic_task_binding(self): + plan = self.controller.propose_plan({ + "title": "Read the page", "summary": "Read and verify the page.", + "steps": [{"id": "read", "title": "Read", "description": "Read src/page.txt.", + "acceptance": [{"criterion": "Page is readable", "verification": { + "action": "read_file", "arguments": {"path": "src/page.txt"}, + }}]}], + }) + self.controller.accept_plan(self.manager, plan_id=plan["plan_id"], version=plan["version"]) + task = self.manager.tasks[0] + result = self.manager.record_evidence( + action="read_file", args='{"path":"src/page.txt"}', result="contents", success=True, + verification_status="verified", + ) + self.assertEqual(result["task_id"], task["id"]) + self.assertEqual(task["checkpoint"]["strategic_plan"]["plan_id"], plan["plan_id"]) + + def test_duplicate_acceptance_racing_with_commit_returns_existing_plan(self): + plan = self._proposal() + winner = InteractionController(self.store, workspace_id="project", session_id="strategic-session") + winner_manager = TaskManager(self.store, workspace_id="project", session_id="strategic-session") + loser_manager = TaskManager(self.store, workspace_id="project", session_id="strategic-session") + original_edit = loser_manager.edit_plan + + def race(**kwargs): + winner.accept_plan(winner_manager, plan_id=plan["plan_id"], version=plan["version"]) + return original_edit(**kwargs) + + with patch.object(loser_manager, "edit_plan", side_effect=race): + duplicate, created = self.controller.accept_plan( + loser_manager, plan_id=plan["plan_id"], version=plan["version"], + ) + self.assertFalse(created) + self.assertEqual(duplicate["working_plan_id"], winner_manager.working_plan.id) + self.assertEqual(len(self.store.list_events( + "interaction.plan_accepted", workspace_id="project", session_id="strategic-session", + )), 1) + + def test_interaction_state_keeps_accepted_bounds_after_generic_event_window(self): + plan = self._proposal() + self.controller.accept_plan(self.manager, plan_id=plan["plan_id"], version=plan["version"]) + now = "2026-10-17T00:00:00Z" + with self.store.database.connection() as db: + db.executemany( + "INSERT INTO events(id,event_type,payload,user_id,workspace_id,session_id,task_id,created_at) " + "VALUES(?,?,?,?,?,?,?,?)", + [(f"noise-{index}", "unrelated.event", "{}", "local", "project", + "strategic-session", None, now) for index in range(10_001)], + ) + restored = InteractionController(self.store, workspace_id="project", session_id="strategic-session") + self.assertEqual(restored.state()["executing_plan"]["fingerprint"], plan["fingerprint"]) + def test_failed_plan_keeps_its_accepted_execution_bounds(self): plan = self._proposal() accepted, _ = self.controller.accept_plan( @@ -218,7 +381,7 @@ def test_failed_plan_can_complete_after_resumed_work_passes_evidence(self): self.controller.complete_plan(accepted, task_manager=self.manager) self.assertIsNone(self.controller.state()["executing_plan"]) - def test_completion_checks_current_working_plan_membership(self): + def test_strategic_working_plan_rejects_unbound_additions(self): plan = self._proposal() accepted, _ = self.controller.accept_plan( self.manager, plan_id=plan["plan_id"], version=plan["version"], @@ -227,12 +390,13 @@ def test_completion_checks_current_working_plan_membership(self): task["status"] = "succeeded" task["evidence"] = [{"success": True, "acceptance": "verified", "acceptance_ids": plan["steps"][0]["acceptance_ids"]}] - self.manager.edit_plan(complexity="complex", plan_id=self.manager.working_plan.id, - expected_revision=self.manager.working_plan.revision, - operations=[{"op": "add", "ref": "extra", "task": { - "description": "Unbound extra work", "acceptance": ["Done"]}}]) - with self.assertRaisesRegex(InteractionError, "linked to the accepted Strategic Plan"): - self.controller.complete_plan(accepted, task_manager=self.manager) + with self.assertRaisesRegex(ValueError, "approved strategic binding"): + self.manager.edit_plan(complexity="complex", plan_id=self.manager.working_plan.id, + expected_revision=self.manager.working_plan.revision, + operations=[{"op": "add", "ref": "extra", "task": { + "description": "Unbound extra work", "acceptance": ["Done"]}}]) + self.controller.complete_plan(accepted, task_manager=self.manager) + self.assertIsNone(self.controller.state()["executing_plan"]) def test_acceptance_atomically_links_working_plan_tasks_and_criteria(self): plan = self._proposal() @@ -1246,16 +1410,23 @@ def turn_end(self, **_kwargs): return None '```json\n{"questions":[{"id":"scope","header":"Scope",' '"prompt":"Which target?","choices":["Core","All"]}]}\n```', 'PlanProposal:\n```json\n{"title":"Marker","summary":"Create a marker safely.",' + '"bounds":{"workspace_paths":["."],"capabilities":["read","write"],' + '"effects":["read_only","filesystem"],"risk_ceiling":"normal"},' '"assumptions":[],"steps":[{"id":"write-marker","title":"Write marker",' - '"description":"Create marker.txt with requested content.","acceptance":["marker.txt exists"]}]}\n```', + '"description":"Create marker.txt with requested content.","acceptance":[{"criterion":"marker.txt exists",' + '"verification":{"action":"read_file","arguments":{"path":"marker.txt"}}}]}]}\n```', 'PlanProposal:\n```json\n{"title":"Marker with verification","summary":"Create and verify a marker.",' + '"bounds":{"workspace_paths":["."],"capabilities":["read","write"],' + '"effects":["read_only","filesystem"],"risk_ceiling":"normal"},' '"assumptions":[],"steps":[{"id":"write-marker","title":"Write marker",' - '"description":"Create marker.txt with requested content.","acceptance":["marker.txt exists"]}]}\n```', + '"description":"Create marker.txt with requested content.","acceptance":[{"criterion":"marker.txt exists",' + '"verification":{"action":"read_file","arguments":{"path":"marker.txt"}}}]}]}\n```', 'Action: {"action":"write_file","args":"marker.txt|ready"}', 'AskUser:\n```json\n{"questions":[{"id":"verify","header":"Verification",' '"prompt":"Which accepted check should finish the task?","choices":["File exists","Read contents"]}]}\n```', 'Action: {"action":"read_file","args":"marker.txt"}', 'TaskDone: 0\nMarker created and verified.', + 'Marker created and verified.', ] try: stream_events = [] From c47a0061bfb289be20a04403ca8390eea9bfa1ef Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 17 Oct 2026 01:13:29 +0800 Subject: [PATCH 04/16] test: align strategic acceptance fixture with bounds --- .../plans/2026-10-16-v3-issue-245.md | 7 +++++ scripts/agent_workflow_acceptance.py | 27 ++++++++++++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index 6a79e87..d0b446f 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -120,3 +120,10 @@ All eight findings were reproduced and fixed; regressions cover each boundary. A - PASS: `git diff --check`; independent manual review found no additional actionable defects in the changed paths. The update remains limited to #245. Leave PR #285 and issue #245 open and unmerged. + +### Stacked CI follow-up + +- CI on the first review-fix head exposed a stale workflow acceptance fixture that proposed mutating work without explicit capability/effect bounds or action verifiers. The Strategic Plan correctly denied those actions, exhausting the scripted response fixture. +- Updated the fixture to declare its workspace, read/write/shell capabilities, filesystem/process effects, and exact execution verifier for each step. +- PASS: exercised the fixture's provider-control normalization, Strategic Plan validation, atomic acceptance, and all four action/argument contracts locally without network or provider calls. +- PENDING: rerun full dispatched CI on the updated final head. The local browser acceptance cannot bind localhost sockets in this sandbox. diff --git a/scripts/agent_workflow_acceptance.py b/scripts/agent_workflow_acceptance.py index 796a1b4..a876f4c 100644 --- a/scripts/agent_workflow_acceptance.py +++ b/scripts/agent_workflow_acceptance.py @@ -90,16 +90,35 @@ def test_page_is_served(self): "plan_name": "Deploy local acceptance page", "overview": "Create a page and verify its local HTTP deployment.", "do_not_execute_until_approved": True, + "bounds": { + "workspace_paths": ["."], + "capabilities": ["read", "write", "shell"], + "effects": ["read_only", "filesystem", "process"], + "risk_ceiling": "normal", + }, "steps": [ {"step_id": "page", "title": "Create index.html", "details": "Write index.html.", - "acceptance_criteria": ["index.html exists"]}, + "acceptance_criteria": [{"criterion": "index.html exists", "verification": { + "action": "write_file", "arguments": {"path": "index.html", "content": initial_page}, + }}]}, {"step_id": "edit", "title": "Edit index.html safely", "details": "Update the draft page before verification.", - "acceptance_criteria": ["index.html contains verified"]}, + "acceptance_criteria": [{"criterion": "index.html contains verified", "verification": { + "action": "edit_file", "arguments": { + "path": "index.html", "old_text": "draft", "new_text": "verified", + "expected_sha256": hashlib.sha256(initial_page.encode()).hexdigest(), + }, + }}]}, {"step_id": "test", "title": "Create test_deploy.py", "details": "Write test_deploy.py.", - "acceptance_criteria": ["test_deploy.py exists"]}, + "acceptance_criteria": [{"criterion": "test_deploy.py exists", "verification": { + "action": "write_file", "arguments": {"path": "test_deploy.py", "content": deployment_test}, + }}]}, {"step_id": "verify", "title": "Verify local deployment", "details": "Run python -m unittest test_deploy.py -v.", - "acceptance_criteria": ["unittest exits successfully"]}, + "acceptance_criteria": [{"criterion": "unittest exits successfully", "verification": { + "action": "run_cmd", "arguments": { + "command": f"{sys.executable} -m unittest test_deploy.py -v", + }, + }}]}, ], } responses = [ From f5a9a6805a3f20fa91e467f03a227a81e844bfce Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 17 Oct 2026 01:18:39 +0800 Subject: [PATCH 05/16] fix: normalize scoped structured mutation arguments --- .../plans/2026-10-16-v3-issue-245.md | 6 ++- openkyrozen/agent/executor.py | 12 +++++- scripts/agent_workflow_acceptance.py | 15 ++++--- tests/test_interaction.py | 39 +++++++++++++++++++ 4 files changed, 62 insertions(+), 10 deletions(-) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index d0b446f..8d6b2cb 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -124,6 +124,8 @@ The update remains limited to #245. Leave PR #285 and issue #245 open and unmerg ### Stacked CI follow-up - CI on the first review-fix head exposed a stale workflow acceptance fixture that proposed mutating work without explicit capability/effect bounds or action verifiers. The Strategic Plan correctly denied those actions, exhausting the scripted response fixture. -- Updated the fixture to declare its workspace, read/write/shell capabilities, filesystem/process effects, and exact execution verifier for each step. -- PASS: exercised the fixture's provider-control normalization, Strategic Plan validation, atomic acceptance, and all four action/argument contracts locally without network or provider calls. +- The next CI run exposed that JSON-string structured `edit_file` arguments were not canonicalized before task-contract matching. The foreground scope check parsed them, but task matching fell through to the original spaced JSON string. +- Canonicalized structured JSON arguments at the shared mutation-matching boundary and added an end-to-end regression. Updated the workflow fixture with explicit bounds and verifiers; it now asks the runtime to read the test harness and runs that generated test from the acceptance harness itself, avoiding an unapproved arbitrary process action. +- PASS: exercised provider-control normalization, Strategic Plan validation, atomic acceptance, and every declared action contract locally without network or provider calls; focused regression passes on Python 3.12 and 3.13. +- PASS: `make check`, `make lint`, and `make docs-check` after the fixture and parser changes. - PENDING: rerun full dispatched CI on the updated final head. The local browser acceptance cannot bind localhost sockets in this sandbox. diff --git a/openkyrozen/agent/executor.py b/openkyrozen/agent/executor.py index 5d5b267..85c712a 100644 --- a/openkyrozen/agent/executor.py +++ b/openkyrozen/agent/executor.py @@ -503,9 +503,17 @@ def emit_rejected_call(): operation_id = self._operation_id(operation_scope, canonical, args) active_plan = self._interaction_controller.state().get("executing_plan") if self._is_state_changing_action(canonical, args) and active_plan: - structured = _legacy_object_arguments(args, expected_spec) + structured = None + if args.lstrip().startswith("{"): + try: + parsed = json.loads(args) + except (json.JSONDecodeError, TypeError): + parsed = None + structured = parsed if isinstance(parsed, dict) else None + if structured is None: + structured = _legacy_object_arguments(args, expected_spec) match_args = (json.dumps(structured, ensure_ascii=False, sort_keys=True, separators=(",", ":")) - if structured else args) + if isinstance(structured, dict) and structured else args) scoped_file_mutation = ( canonical in {"write_file", "edit_file"} and _file_mutation_matches_objective(next((task for task in self.tasks.tasks diff --git a/scripts/agent_workflow_acceptance.py b/scripts/agent_workflow_acceptance.py index a876f4c..6c16dea 100644 --- a/scripts/agent_workflow_acceptance.py +++ b/scripts/agent_workflow_acceptance.py @@ -7,6 +7,7 @@ import hashlib import os import socket +import subprocess import sys import tempfile from pathlib import Path @@ -113,11 +114,9 @@ def test_page_is_served(self): "action": "write_file", "arguments": {"path": "test_deploy.py", "content": deployment_test}, }}]}, {"step_id": "verify", "title": "Verify local deployment", - "details": "Run python -m unittest test_deploy.py -v.", + "details": "Read the deployment test; the acceptance harness runs it after the plan finishes.", "acceptance_criteria": [{"criterion": "unittest exits successfully", "verification": { - "action": "run_cmd", "arguments": { - "command": f"{sys.executable} -m unittest test_deploy.py -v", - }, + "action": "read_file", "arguments": {"path": "test_deploy.py"}, }}]}, ], } @@ -135,7 +134,7 @@ def test_page_is_served(self): {"action": "write_file", "args": f"test_deploy.py|{deployment_test}"} ), "TaskDone: 2\nAction: " + json.dumps( - {"action": "run_cmd", "args": f"{sys.executable} -m unittest test_deploy.py -v"} + {"action": "read_file", "args": "test_deploy.py"} ), "TaskDone: 3\nDeployment verified.", ] @@ -178,11 +177,15 @@ def test_page_is_served(self): assert "Deployment verified" in reply, reply assert [task["status"] for task in main.tasks.tasks] == ["succeeded"] * 4 + subprocess.run( + [sys.executable, "-m", "unittest", "test_deploy.py", "-v"], + cwd=root, check=True, capture_output=True, text=True, + ) receipts = store.list_events( "execution.receipt", workspace_id="acceptance", session_id="release", ) assert [event["payload"]["action"] for event in reversed(receipts)] == [ - "list_dir", "write_file", "edit_file", "write_file", "run_cmd", + "list_dir", "write_file", "edit_file", "write_file", "read_file", ] state = main._interaction_controller.state() assert state["executing_plan"] is None diff --git a/tests/test_interaction.py b/tests/test_interaction.py index 718c6c2..ca2460c 100644 --- a/tests/test_interaction.py +++ b/tests/test_interaction.py @@ -1102,6 +1102,45 @@ def test_bounded_write_objective_allows_mutation_with_a_separate_read_verifier(s self.agent.tasks = previous_tasks self.agent._set_workspace_root(previous_root) + def test_structured_legacy_arguments_match_the_accepted_action_contract(self): + from openkyrozen.agent.types import ExecutionReceipt + previous = self.agent._interaction_controller + previous_tasks = self.agent.tasks + previous_root = self.agent._get_workspace_root() + manager = TaskManager(self.store, workspace_id="workspace", session_id="session") + controller = InteractionController(self.store, workspace_id="workspace", session_id="session") + arguments = {"path": "src/file.py", "old_text": "draft", "new_text": "verified", + "expected_sha256": "a" * 64} + proposal = controller.propose_plan({ + "title": "Update source", "summary": "Update the source file.", "bounds": { + "workspace_paths": ["src"], "capabilities": ["write"], + "effects": ["filesystem"], "risk_ceiling": "normal", + }, "steps": [{"id": "edit", "title": "Update source", "description": "Update the source file.", + "acceptance": [{"criterion": "Source is updated", "verification": { + "action": "edit_file", "arguments": arguments, + }}]}], + }) + controller.accept_plan(manager, plan_id=proposal["plan_id"], version=proposal["version"]) + self.agent.tasks = manager + self.agent._interaction_controller = controller + self.agent._set_workspace_root(Path(self.directory.name)) + receipt = ExecutionReceipt( + "receipt", "operation", "edit_file", json.dumps(arguments), True, + "start", "end", True, "edited", call_id="call", call_id_source="synthetic", + arguments=arguments, tool_result=ToolResult(text="edited"), + ) + try: + with patch.object(self.agent, "_run_tool", return_value=receipt) as run_tool: + result = self.agent._execute_turn_action( + "edit_file", json.dumps(arguments), operation_scope="test", successful_operations=set(), + ) + self.assertTrue(result.success, result.result) + run_tool.assert_called_once() + finally: + self.agent._interaction_controller = previous + self.agent.tasks = previous_tasks + self.agent._set_workspace_root(previous_root) + def test_plan_requires_inspection_and_normalizes_structured_prose(self): class LearningStub: def feedback_signal(self, _text): return None From 3ccb6c7ece57400d4d9110c85ddb33140fa7da86 Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 17 Oct 2026 01:21:46 +0800 Subject: [PATCH 06/16] test: correct strategic workflow task sequence --- docs/superpowers/plans/2026-10-16-v3-issue-245.md | 1 + scripts/agent_workflow_acceptance.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index 8d6b2cb..49d1319 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -126,6 +126,7 @@ The update remains limited to #245. Leave PR #285 and issue #245 open and unmerg - CI on the first review-fix head exposed a stale workflow acceptance fixture that proposed mutating work without explicit capability/effect bounds or action verifiers. The Strategic Plan correctly denied those actions, exhausting the scripted response fixture. - The next CI run exposed that JSON-string structured `edit_file` arguments were not canonicalized before task-contract matching. The foreground scope check parsed them, but task matching fell through to the original spaced JSON string. - Canonicalized structured JSON arguments at the shared mutation-matching boundary and added an end-to-end regression. Updated the workflow fixture with explicit bounds and verifiers; it now asks the runtime to read the test harness and runs that generated test from the acceptance harness itself, avoiding an unapproved arbitrary process action. +- The following CI run advanced through the structured edit and caught a stale `TaskDone` index in the fixture. Corrected the task sequence before the next run. - PASS: exercised provider-control normalization, Strategic Plan validation, atomic acceptance, and every declared action contract locally without network or provider calls; focused regression passes on Python 3.12 and 3.13. - PASS: `make check`, `make lint`, and `make docs-check` after the fixture and parser changes. - PENDING: rerun full dispatched CI on the updated final head. The local browser acceptance cannot bind localhost sockets in this sandbox. diff --git a/scripts/agent_workflow_acceptance.py b/scripts/agent_workflow_acceptance.py index 6c16dea..d29a0d8 100644 --- a/scripts/agent_workflow_acceptance.py +++ b/scripts/agent_workflow_acceptance.py @@ -130,7 +130,7 @@ def test_page_is_served(self): "expected_sha256": hashlib.sha256(initial_page.encode()).hexdigest(), }), }), - "TaskDone: 0\nAction: " + json.dumps( + "TaskDone: 1\nAction: " + json.dumps( {"action": "write_file", "args": f"test_deploy.py|{deployment_test}"} ), "TaskDone: 2\nAction: " + json.dumps( From 178c0bf6d44f77246d24a68c7337e80767d7b6d9 Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 17 Oct 2026 01:26:21 +0800 Subject: [PATCH 07/16] test: verify strategic edits before task completion --- docs/superpowers/plans/2026-10-16-v3-issue-245.md | 2 +- scripts/agent_workflow_acceptance.py | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index 49d1319..d1f5e0d 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -126,7 +126,7 @@ The update remains limited to #245. Leave PR #285 and issue #245 open and unmerg - CI on the first review-fix head exposed a stale workflow acceptance fixture that proposed mutating work without explicit capability/effect bounds or action verifiers. The Strategic Plan correctly denied those actions, exhausting the scripted response fixture. - The next CI run exposed that JSON-string structured `edit_file` arguments were not canonicalized before task-contract matching. The foreground scope check parsed them, but task matching fell through to the original spaced JSON string. - Canonicalized structured JSON arguments at the shared mutation-matching boundary and added an end-to-end regression. Updated the workflow fixture with explicit bounds and verifiers; it now asks the runtime to read the test harness and runs that generated test from the acceptance harness itself, avoiding an unapproved arbitrary process action. -- The following CI run advanced through the structured edit and caught a stale `TaskDone` index in the fixture. Corrected the task sequence before the next run. +- The following CI run advanced through the structured edit and caught a stale `TaskDone` index in the fixture. Corrected the task sequence; the next run then showed that an edit receipt alone does not establish the file-content criterion, so the fixture now reads the edited file before completing that task. - PASS: exercised provider-control normalization, Strategic Plan validation, atomic acceptance, and every declared action contract locally without network or provider calls; focused regression passes on Python 3.12 and 3.13. - PASS: `make check`, `make lint`, and `make docs-check` after the fixture and parser changes. - PENDING: rerun full dispatched CI on the updated final head. The local browser acceptance cannot bind localhost sockets in this sandbox. diff --git a/scripts/agent_workflow_acceptance.py b/scripts/agent_workflow_acceptance.py index d29a0d8..eff9de4 100644 --- a/scripts/agent_workflow_acceptance.py +++ b/scripts/agent_workflow_acceptance.py @@ -104,10 +104,7 @@ def test_page_is_served(self): }}]}, {"step_id": "edit", "title": "Edit index.html safely", "details": "Update the draft page before verification.", "acceptance_criteria": [{"criterion": "index.html contains verified", "verification": { - "action": "edit_file", "arguments": { - "path": "index.html", "old_text": "draft", "new_text": "verified", - "expected_sha256": hashlib.sha256(initial_page.encode()).hexdigest(), - }, + "action": "read_file", "arguments": {"path": "index.html"}, }}]}, {"step_id": "test", "title": "Create test_deploy.py", "details": "Write test_deploy.py.", "acceptance_criteria": [{"criterion": "test_deploy.py exists", "verification": { @@ -130,6 +127,7 @@ def test_page_is_served(self): "expected_sha256": hashlib.sha256(initial_page.encode()).hexdigest(), }), }), + "Action: " + json.dumps({"action": "read_file", "args": "index.html"}), "TaskDone: 1\nAction: " + json.dumps( {"action": "write_file", "args": f"test_deploy.py|{deployment_test}"} ), @@ -185,7 +183,7 @@ def test_page_is_served(self): "execution.receipt", workspace_id="acceptance", session_id="release", ) assert [event["payload"]["action"] for event in reversed(receipts)] == [ - "list_dir", "write_file", "edit_file", "write_file", "read_file", + "list_dir", "write_file", "edit_file", "read_file", "write_file", "read_file", ] state = main._interaction_controller.state() assert state["executing_plan"] is None From de1f6d1742da7ea6113f1881de3acb72968df142 Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 17 Oct 2026 01:35:09 +0800 Subject: [PATCH 08/16] fix: preserve legacy receipt matching and operation identity --- .../plans/2026-10-16-v3-issue-245.md | 4 +- openkyrozen/agent/executor.py | 45 ++++++++++++------- 2 files changed, 33 insertions(+), 16 deletions(-) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index d1f5e0d..083f9dd 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -127,6 +127,8 @@ The update remains limited to #245. Leave PR #285 and issue #245 open and unmerg - The next CI run exposed that JSON-string structured `edit_file` arguments were not canonicalized before task-contract matching. The foreground scope check parsed them, but task matching fell through to the original spaced JSON string. - Canonicalized structured JSON arguments at the shared mutation-matching boundary and added an end-to-end regression. Updated the workflow fixture with explicit bounds and verifiers; it now asks the runtime to read the test harness and runs that generated test from the acceptance harness itself, avoiding an unapproved arbitrary process action. - The following CI run advanced through the structured edit and caught a stale `TaskDone` index in the fixture. Corrected the task sequence; the next run then showed that an edit receipt alone does not establish the file-content criterion, so the fixture now reads the edited file before completing that task. -- PASS: exercised provider-control normalization, Strategic Plan validation, atomic acceptance, and every declared action contract locally without network or provider calls; focused regression passes on Python 3.12 and 3.13. +- Final CI for that head passed the wheel acceptance and reported two task-consistency errors plus the previously observed delegation failure. The task errors came from a mismatch between canonical receipt arguments and legacy task checkpoints: initial receipts did not reconcile, and duplicate-operation IDs differed between successful and rejected receipts. +- The execution boundary now matches legacy checkpoint strings only for pending legacy tasks while preserving canonical fingerprints for structured contracts; duplicate detection and rejected receipts share the canonical operation identity. Both task-consistency regressions pass locally. +- PASS: interaction, Working Plan, receipt, and task-consistency suites — 171 tests on Python 3.12 and 171 on Python 3.13. Workflow normalization, Strategic Plan validation, atomic acceptance, and every declared action contract also pass locally without network or provider calls. - PASS: `make check`, `make lint`, and `make docs-check` after the fixture and parser changes. - PENDING: rerun full dispatched CI on the updated final head. The local browser acceptance cannot bind localhost sockets in this sandbox. diff --git a/openkyrozen/agent/executor.py b/openkyrozen/agent/executor.py index 85c712a..7b28eda 100644 --- a/openkyrozen/agent/executor.py +++ b/openkyrozen/agent/executor.py @@ -500,18 +500,19 @@ def emit_rejected_call(): expected_spec=expected_spec, ) args = str(args) - operation_id = self._operation_id(operation_scope, canonical, args) + structured = None + if args.lstrip().startswith("{"): + try: + parsed = json.loads(args) + except (json.JSONDecodeError, TypeError): + parsed = None + structured = parsed if isinstance(parsed, dict) else None + if structured is None: + structured = _legacy_object_arguments(args, expected_spec) + operation_arguments = structured if isinstance(structured, dict) and structured else args + operation_id = self._operation_id(operation_scope, canonical, operation_arguments) active_plan = self._interaction_controller.state().get("executing_plan") if self._is_state_changing_action(canonical, args) and active_plan: - structured = None - if args.lstrip().startswith("{"): - try: - parsed = json.loads(args) - except (json.JSONDecodeError, TypeError): - parsed = None - structured = parsed if isinstance(parsed, dict) else None - if structured is None: - structured = _legacy_object_arguments(args, expected_spec) match_args = (json.dumps(structured, ensure_ascii=False, sort_keys=True, separators=(",", ":")) if isinstance(structured, dict) and structured else args) scoped_file_mutation = ( @@ -531,7 +532,9 @@ def emit_rejected_call(): outcome = tool_failure("permission_denied", result) self._notify_tool_execute(canonical, args, outcome) return self._make_execution_receipt( - action=canonical, args=args, authorized=False, started_at=started_at, + action=canonical, args=args, + receipt_arguments=operation_arguments if isinstance(operation_arguments, dict) else None, + authorized=False, started_at=started_at, success=False, result=result, operation_scope=operation_scope, failure=outcome.error.code, tool_error=outcome.error, tool_result=outcome, call_id=call_id, call_id_source="synthetic", @@ -544,7 +547,9 @@ def emit_rejected_call(): result = render_tool_result(outcome) self._notify_tool_execute(canonical, args, outcome) return self._make_execution_receipt( - action=canonical, args=args, authorized=True, started_at=started_at, success=False, + action=canonical, args=args, + receipt_arguments=operation_arguments if isinstance(operation_arguments, dict) else None, + authorized=True, started_at=started_at, success=False, result=result, operation_scope=operation_scope, failure=outcome.error.code, tool_error=outcome.error, tool_result=outcome, call_id=call_id, call_id_source="synthetic", expected_spec=expected_spec, @@ -578,12 +583,22 @@ def _record_turn_receipt(self, receipt: ExecutionReceipt, *, allow_legacy_task_m bool(explicit_task_id) or receipt.call_id_source == "legacy" or (allow_legacy_task_matching and receipt.call_id_source == "synthetic") ) - matching_args = (json.dumps(receipt.arguments, ensure_ascii=False, allow_nan=False, - sort_keys=True, separators=(",", ":")) + legacy_contract = any( + task.get("status") in {"pending", "running"} + and self.tasks._dependencies_succeeded(task, self.tasks.tasks) + and (task.get("checkpoint") or {}).get("action") == receipt.action + and isinstance((task.get("checkpoint") or {}).get("args"), str) + and not task["checkpoint"]["args"].lstrip().startswith("{") + for task in self.tasks.tasks + ) + matching_args = (receipt.args if legacy_contract else + json.dumps(receipt.arguments, ensure_ascii=False, allow_nan=False, + sort_keys=True, separators=(",", ":")) if receipt.arguments is not None else receipt.args) evidence = {} if receipt.action in TOOLS or not should_reconcile else self.tasks.record_evidence( task_id=explicit_task_id, action=receipt.action, args=matching_args, - args_projection=receipt_projection["args"], args_fingerprint=receipt.args_sha256, + args_projection=receipt_projection["args"], + args_fingerprint=None if legacy_contract else receipt.args_sha256, result=receipt_projection["result"], success=receipt.success, acceptance=task_acceptance, receipt_id=receipt.receipt_id, verification_status=receipt.verification.get("status"), From 72142a4dee88103a39c728697d8309a2f918d92b Mon Sep 17 00:00:00 2001 From: Evan Gong Date: Sat, 17 Oct 2026 08:45:36 +0800 Subject: [PATCH 09/16] fix: bind delegated writes to approved strategic contracts --- .../plans/2026-10-16-v3-issue-245.md | 10 +++++ docs/tasks-and-automation.md | 2 +- openkyrozen/agent/delegation_runtime.py | 21 +++++++-- openkyrozen/agent/modes.py | 42 ++++++++++++++++-- openkyrozen/agent/prompts.py | 4 +- openkyrozen/tasks/engine.py | 16 ++++++- tests/test_delegation.py | 15 +++++-- tests/test_interaction.py | 44 +++++++++++++++++-- 8 files changed, 137 insertions(+), 17 deletions(-) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index 083f9dd..ca6efbf 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -132,3 +132,13 @@ The update remains limited to #245. Leave PR #285 and issue #245 open and unmerg - PASS: interaction, Working Plan, receipt, and task-consistency suites — 171 tests on Python 3.12 and 171 on Python 3.13. Workflow normalization, Strategic Plan validation, atomic acceptance, and every declared action contract also pass locally without network or provider calls. - PASS: `make check`, `make lint`, and `make docs-check` after the fixture and parser changes. - PENDING: rerun full dispatched CI on the updated final head. The local browser acceptance cannot bind localhost sockets in this sandbox. + +### Delegated legacy-action CI regression + +- [x] Reproduced `test_delegated_receipts_do_not_infer_strategic_plan_task_association` passing on the #242 base branch and failing on the current #245 head. +- [x] Add an optional, user-visible structured `write_file`/`edit_file` contract to a Strategic Plan objective. Bind it into the plan fingerprint and linked task checkpoint; require exact action/arguments for delegated matching, without inferring or attaching a task ID to the child receipt. +- [x] Recheck the parent Strategic Plan's capability, effect, risk, and workspace bounds before allowing delegated file operations. +- [x] Rerun the focused regression and plan/delegation suites on Python 3.12; same-basename/different-directory writes remain blocked and the successful child receipt has no inferred task ID. +- PASS: interaction, Working Plan, receipt, task-consistency, and delegation suites — 213 tests on Python 3.12; `make check`, `make lint`, and `make docs-check` pass. +- BLOCKED: local Python 3.13 focused suite could not be provisioned because PyPI DNS/network access is unavailable; CI will provide supported-runtime validation. +- [ ] Rerun final-head CI and obtain independent automated review before beginning #246. diff --git a/docs/tasks-and-automation.md b/docs/tasks-and-automation.md index 9360a19..5e15e18 100644 --- a/docs/tasks-and-automation.md +++ b/docs/tasks-and-automation.md @@ -13,7 +13,7 @@ Task IDs and records are actor/workspace/session scoped. The web task API expose Simple work does not require a plan, and a plan is optional for medium work. Complex direct work uses a durable Working Plan with runtime-owned stable IDs and explicit acceptance criteria. The typed `update_plan` boundary applies revision-checked batches of add, update, cancel, reorder, and dependency operations; legacy `complexity`/`tasks` input is adapted through the same validation path. A changed task must still be pending and unexecuted; executed task records and evidence are preserved, and changed work is represented by a new task after cancellation. Scheduling follows plan order and waits for every dependency to succeed. A plan completes only when every task succeeds with acceptance evidence; recovery also reconciles a crash between the final task write and the plan status write. Clearing an active plan cancels its unfinished steps. The existing foreground text protocol continues to use Plan/TaskList compatibility until the native foreground loop is migrated. -Plan Mode can produce a versioned Strategic Plan before execution. Acceptance records the exact objectives, acceptance criteria, workspace paths, capabilities, effect categories, and risk ceiling alongside its linked Working Plan. Each criterion should include a `verification` action and exact object arguments; only a successful receipt matching that contract records the criterion ID as verified. Plain-text criteria remain readable for legacy proposals but cannot satisfy the per-criterion completion gate without a verifier. Runtime tool dispatch checks each tool's catalog policy and supplied paths against those accepted bounds; shell processes are denied because this runtime cannot confine arbitrary commands to those paths. An unbounded legacy proposal receives read-only defaults. Strategic acceptance does not grant capabilities, bypass tool approvals, or establish completion evidence. +Plan Mode can produce a versioned Strategic Plan before execution. Acceptance records the exact objectives, acceptance criteria, workspace paths, capabilities, effect categories, and risk ceiling alongside its linked Working Plan. A step may also carry a visible, fingerprinted `execution` contract for one exact `write_file` or `edit_file` call; delegated file mutations require this explicit contract and still pass assigned-file, workspace-bound, and tool-approval checks. Each criterion should include a `verification` action and exact object arguments; only a successful receipt matching that contract records the criterion ID as verified. Plain-text criteria remain readable for legacy proposals but cannot satisfy the per-criterion completion gate without a verifier. Runtime tool dispatch checks each tool's catalog policy and supplied paths against those accepted bounds; shell processes are denied because this runtime cannot confine arbitrary commands to those paths. An unbounded legacy proposal receives read-only defaults. Strategic acceptance does not grant capabilities, bypass tool approvals, or establish completion evidence. For a typed plan step, include an optional `execution` object with the canonical tool `action` and object `arguments`. Native tool calls associate with a task only when the runtime receives that task's explicit ID; matching action and arguments alone never select a task. A successful associated receipt may add execution evidence, but it cannot satisfy acceptance criteria by itself. A delegated assignment may carry that explicit Working Plan task ID; the parent records task evidence only after the independent review succeeds. The legacy text adapter retains its bounded matching behavior for compatibility. Plan IDs and task IDs are returned as structured result data so later updates can retain runtime-owned task identities. diff --git a/openkyrozen/agent/delegation_runtime.py b/openkyrozen/agent/delegation_runtime.py index 91ce74d..b022d9c 100644 --- a/openkyrozen/agent/delegation_runtime.py +++ b/openkyrozen/agent/delegation_runtime.py @@ -361,9 +361,24 @@ def _delegation_tool_access(self, action, args): if path not in owned: raise ValueError("Sub-agent write is outside its assigned files") parent = getattr(self.current_session, "_delegation_parent", None) - if parent and parent.interaction.state().get("executing_plan") and self._is_state_changing_action(action, args): - structured = json.dumps(args, ensure_ascii=False, sort_keys=True) if isinstance(args, dict) else str(args) - allowed, reason = parent.tasks.mutation_matches_current_task(action, structured) + parent_plan = parent.interaction.state().get("executing_plan") if parent else None + if parent and parent_plan and self._is_state_changing_action(action, args): + from openkyrozen.agent.executor import _legacy_object_arguments, _strategic_plan_denial + arguments = args if isinstance(args, dict) else None + spec = getattr(self.tool_registry, "specs", {}).get(action) + if arguments is None and isinstance(args, str): + if spec is not None: + normalized = _legacy_object_arguments(args, spec) + arguments = normalized if normalized else None + match_args = (json.dumps(arguments, ensure_ascii=False, sort_keys=True, separators=(",", ":")) + if arguments is not None else str(args)) + denial = _strategic_plan_denial( + parent_plan, action, arguments if arguments is not None else args, + spec, parent.workspace.root, + ) + if denial: + raise PermissionError(f"Sub-agent operation exceeds the accepted Strategic Plan: {denial}") + allowed, reason = parent.tasks.mutation_matches_current_task(action, match_args) if not allowed: raise ValueError("Sub-agent action does not match accepted plan: " + reason) guarded = action in {"read_file", "write_file", "edit_file", "search_files"} or self._is_state_changing_action(action, args) or action in {"git_diff", "git_show"} diff --git a/openkyrozen/agent/modes.py b/openkyrozen/agent/modes.py index df9fb82..687bc62 100644 --- a/openkyrozen/agent/modes.py +++ b/openkyrozen/agent/modes.py @@ -8,6 +8,7 @@ import re import shlex import uuid +from copy import deepcopy from typing import Any from .ports import InteractionStore @@ -184,6 +185,27 @@ def validate_plan_proposal(value: Any, *, previous: dict[str, Any] | None = None if step_id in seen_ids: raise InteractionError("plan step ids must be unique") seen_ids.add(step_id) + execution = step.get("execution") + if execution is not None: + action = execution.get("action") if isinstance(execution, dict) else None + arguments = execution.get("arguments") if isinstance(execution, dict) else None + if (not isinstance(execution, dict) or set(execution) != {"action", "arguments"} + or not isinstance(action, str) or action not in {"write_file", "edit_file"} + or not isinstance(arguments, dict) + or not isinstance(arguments.get("path"), str) or not arguments["path"].strip()): + raise InteractionError("file execution contract must name write_file or edit_file and an exact path") + required = ({"path", "content"} if action == "write_file" else + {"path", "old_text", "new_text", "expected_sha256"}) + if set(arguments) != required or any(not isinstance(item, str) for item in arguments.values()): + raise InteractionError("file execution arguments must exactly match the selected file operation") + if action == "edit_file" and ( + not arguments["old_text"] or not re.fullmatch(r"[0-9a-fA-F]{64}", arguments["expected_sha256"])): + raise InteractionError("edit_file requires nonempty old_text and a SHA-256 read hash") + try: + json.dumps(arguments, ensure_ascii=False, allow_nan=False) + except (TypeError, ValueError) as exc: + raise InteractionError("file execution arguments must be JSON-safe") from exc + execution = {"action": action, "arguments": deepcopy(arguments)} acceptance = step.get("acceptance") if isinstance(acceptance, str): acceptance = [acceptance] @@ -220,14 +242,17 @@ def validate_plan_proposal(value: Any, *, previous: dict[str, Any] | None = None acceptance_ids.append(f"{step_id}-ac-{hashlib.sha256(identity.encode('utf-8')).hexdigest()[:12]}") if len(set(acceptance_ids)) != len(acceptance_ids): raise InteractionError("acceptance criteria must be unique within each objective") - normalised.append({ + normalised_step = { "id": step_id, "title": _text(step.get("title") or f"Step {index}", "step title", 120), "description": _text(step.get("description"), "step description", 800), "acceptance": acceptance, "acceptance_ids": acceptance_ids, "acceptance_verifications": acceptance_verifications, - }) + } + if execution is not None: + normalised_step["execution"] = execution + normalised.append(normalised_step) assumptions = value.get("assumptions", []) if assumptions is None: assumptions = [] @@ -373,6 +398,7 @@ def normalize_provider_control(text: str, *, plan_mode: bool = False) -> tuple[s step.get("acceptance") or step.get("acceptance_criteria") or ["Step completed with observable evidence"] ), + **({"execution": step["execution"]} if "execution" in step else {}), }) return "PlanProposal", { "title": value.get("title") or value.get("plan_name"), @@ -625,19 +651,26 @@ def accept_plan(self, task_manager: Any, *, plan_id: str | None = None, and (task.get("checkpoint") or {}).get("strategic_plan", {}).get("acceptance_ids") == step["acceptance_ids"] and task.get("description") == description and [item.get("criterion") if isinstance(item, dict) else item - for item in task.get("acceptance", [])] == step["acceptance"]), None) + for item in task.get("acceptance", [])] == step["acceptance"] + and (task.get("checkpoint") or {}).get("action") == (step.get("execution") or {}).get("action") + and ((task.get("checkpoint") or {}).get("args") or "") == ( + json.dumps((step.get("execution") or {}).get("arguments", {}), ensure_ascii=False, + allow_nan=False, sort_keys=True, separators=(",", ":")) + if step.get("execution") else "")), None) if existing: reused.add(existing["id"]) continue operations.append({"op": "add", "ref": step["id"], "task": { "description": description, "acceptance": step["acceptance"], + **({"execution": step["execution"]} if step.get("execution") else {}), }}) strategic_bindings[step["id"]] = { "plan_id": plan["plan_id"], "version": plan["version"], "fingerprint": plan["fingerprint"], "objective_id": step["id"], "acceptance_ids": step["acceptance_ids"], "acceptance_verifications": step["acceptance_verifications"], + "execution": step.get("execution"), } for task in previous_tasks: if task["id"] not in reused and task.get("status") in {"pending", "failed", "blocked"}: @@ -753,6 +786,9 @@ def render_plan(plan: dict[str, Any]) -> str: lines.append(f"{index}. {step['title']} — {step['description']}") lines.extend(f" - Acceptance [{criterion_id}]: {item}" for criterion_id, item in zip(step["acceptance_ids"], step["acceptance"])) + if step.get("execution"): + lines.append(f" - Approved file operation: {step['execution']['action']} " + f"{json.dumps(step['execution']['arguments'], ensure_ascii=False, sort_keys=True)}") for verification in step["acceptance_verifications"]: if verification: lines.append(f" Verified by {verification['action']} with the approved arguments.") diff --git a/openkyrozen/agent/prompts.py b/openkyrozen/agent/prompts.py index ecbceaa..796ff43 100644 --- a/openkyrozen/agent/prompts.py +++ b/openkyrozen/agent/prompts.py @@ -197,8 +197,8 @@ def _system_prompt(self, tools_list: str, agent_config: dict[str, Any] | None = + ( "The final plan must use this standalone control:\n" "PlanProposal:\n```json\n" - '{"title":"Plan title","summary":"Outcome","assumptions":[],"bounds":{"workspace_paths":["src"],"capabilities":["read","write"],"effects":["read_only","filesystem"],"risk_ceiling":"normal"},"steps":[{"id":"objective-1","title":"Step","description":"Work to perform after acceptance","acceptance":[{"criterion":"Observable result","verification":{"action":"read_file","arguments":{"path":"src/result.txt"}}}]}]}\n' - "```\nUse 1-10 stable objective IDs and state the smallest workspace paths, capability/effect categories, and normal or high risk ceiling. Output no task or execution controls.\n" + '{"title":"Plan title","summary":"Outcome","assumptions":[],"bounds":{"workspace_paths":["src"],"capabilities":["read","write"],"effects":["read_only","filesystem"],"risk_ceiling":"normal"},"steps":[{"id":"objective-1","title":"Step","description":"Work to perform after acceptance","acceptance":[{"criterion":"Observable result","verification":{"action":"read_file","arguments":{"path":"src/result.txt"}}}],"execution":{"action":"write_file","arguments":{"path":"src/result.txt","content":"approved content"}}}]}\n' + "```\nUse 1-10 stable objective IDs and state the smallest workspace paths, capability/effect categories, and normal or high risk ceiling. Include an optional exact `execution` object only for a planned file write/edit; its action and complete arguments are part of user approval, and it does not execute before acceptance. Do not output task controls or tool calls.\n" if interaction_mode == "plan" else "" ) + "\n## Available read-only tools\n" + readonly_tools + "\n\n" diff --git a/openkyrozen/tasks/engine.py b/openkyrozen/tasks/engine.py index d4931d7..2241850 100644 --- a/openkyrozen/tasks/engine.py +++ b/openkyrozen/tasks/engine.py @@ -280,8 +280,10 @@ def resolve(value: Any) -> str: if strategic_bindings and ref in strategic_bindings: binding = strategic_bindings[ref] if (not isinstance(binding, dict) - or set(binding) != {"plan_id", "version", "fingerprint", "objective_id", "acceptance_ids", - "acceptance_verifications"} + or set(binding) not in ({"plan_id", "version", "fingerprint", "objective_id", "acceptance_ids", + "acceptance_verifications"}, + {"plan_id", "version", "fingerprint", "objective_id", "acceptance_ids", + "acceptance_verifications", "execution"}) or not isinstance(binding.get("version"), int) or not isinstance(binding.get("acceptance_ids"), list) or not isinstance(binding.get("acceptance_verifications"), list)): @@ -291,6 +293,16 @@ def resolve(value: Any) -> str: or any(binding.get(key) != accepted_payload.get(key) for key in ("plan_id", "version", "fingerprint"))): raise ValueError("strategic task binding must match the approved plan") + execution = binding.get("execution") + if execution is not None: + if (not isinstance(execution, dict) or set(execution) != {"action", "arguments"} + or fields["checkpoint"].get("action") != _receipt_action(execution.get("action")) + or fields["checkpoint"].get("args") != json.dumps( + execution.get("arguments"), ensure_ascii=False, allow_nan=False, + sort_keys=True, separators=(",", ":"))): + raise ValueError("strategic task execution must match its approved contract") + elif fields["checkpoint"].get("action"): + raise ValueError("strategic task execution is missing from its approved binding") task["checkpoint"] = {**task["checkpoint"], "strategic_plan": deepcopy(binding)} verifications = binding["acceptance_verifications"] if len(verifications) != len(fields["acceptance"]): diff --git a/tests/test_delegation.py b/tests/test_delegation.py index 712462b..4976887 100644 --- a/tests/test_delegation.py +++ b/tests/test_delegation.py @@ -888,13 +888,18 @@ def call(name, args, session="mcp-a"): def test_delegated_receipts_do_not_infer_strategic_plan_task_association(self): runtime = self.runtime proposal = runtime.current_session.interaction.propose_plan({"title":"Create two files", - "summary":"Write only first.txt and second.txt", "assumptions":[], "steps":[ + "summary":"Write only first.txt and second.txt", "assumptions":[], + "bounds":{"workspace_paths":["."],"capabilities":["read","write"], + "effects":["read_only","filesystem"],"risk_ceiling":"normal"}, "steps":[ {"id": "first", "title":"Create first.txt", "description":"Write first.txt with ok", - "acceptance":["first.txt exists"]}, + "acceptance":["first.txt exists"], + "execution":{"action":"write_file","arguments":{"path":"first.txt","content":"ok"}}}, {"id": "second", "title":"Create second.txt", "description":"Write second.txt with ok", - "acceptance":["second.txt exists"]}]}) + "acceptance":["second.txt exists"], + "execution":{"action":"write_file","arguments":{"path":"second.txt","content":"ok"}}}]}) runtime.current_session.interaction.accept_plan(runtime.tasks, plan_id=proposal["plan_id"], version=proposal["version"]) + self.assertEqual(runtime.tasks.tasks[0]["checkpoint"].get("action"), "write_file") class Provider: def chat(inner, messages, model): reviewing = "Independently verify" in messages[1]["content"] @@ -914,12 +919,16 @@ def chat(inner, messages, model): wrong = coordinator.spawn([brief("wrong", profile="coder", scope=["outside-plan.txt"])])[0] self.assertEqual(coordinator.wait([wrong["run_id"]],5)[0]["status"], "blocked") self.assertFalse((self.root/"outside-plan.txt").exists()) + same_name = coordinator.spawn([brief("same-name", profile="coder", scope=["other/first.txt"])])[0] + self.assertEqual(coordinator.wait([same_name["run_id"]],5)[0]["status"], "blocked") + self.assertFalse((self.root/"other/first.txt").exists()) agents = coordinator.spawn([brief(0, profile="coder", scope=["first.txt"]), brief(1, profile="coder", scope=["second.txt"], dependencies=["0"])]) finished = coordinator.wait([run["run_id"] for run in agents],5) finally: runtime._active_interaction_mode.reset(token) self.assertEqual([run["status"] for run in finished], ["succeeded", "blocked"], finished) + self.assertIsNone(finished[0]["result"]["tool_records"][0]["task_id"]) self.assertFalse((self.root/"second.txt").exists()) def test_explicit_working_plan_task_id_reconciles_only_after_independent_review(self): diff --git a/tests/test_interaction.py b/tests/test_interaction.py index ca2460c..59cc589 100644 --- a/tests/test_interaction.py +++ b/tests/test_interaction.py @@ -145,6 +145,7 @@ def test_revalidating_a_normalized_plan_preserves_verified_acceptance_contracts( proposal = { "title": "Change", "summary": "Make the change.", "steps": [{"id": "inspect", "title": "Inspect", "description": "Read the file.", + "execution": {"action": "write_file", "arguments": {"path": "src/file.txt", "content": "x"}}, "acceptance": [{"criterion": "File is readable", "verification": { "action": "read_file", "arguments": {"path": "src/file.txt"}, }}]}], @@ -153,6 +154,7 @@ def test_revalidating_a_normalized_plan_preserves_verified_acceptance_contracts( second = validate_plan_proposal(first) self.assertEqual(second["steps"][0]["acceptance_verifications"], first["steps"][0]["acceptance_verifications"]) + self.assertEqual(second["steps"][0]["execution"], first["steps"][0]["execution"]) self.assertEqual(second["steps"][0]["acceptance_ids"], first["steps"][0]["acceptance_ids"]) def test_strategic_plan_has_stable_objective_criteria_and_approved_bounds(self): @@ -169,10 +171,14 @@ def test_strategic_plan_has_stable_objective_criteria_and_approved_bounds(self): first = validate_plan_proposal(proposal) repeated = validate_plan_proposal(proposal) revised = validate_plan_proposal({**proposal, "summary": "Deliver the requested change."}, previous=first) + changed_operation = validate_plan_proposal({**proposal, "steps": [{**proposal["steps"][0], + "execution": {"action": "write_file", "arguments": {"path": "src/file.txt", "content": "different"}}, + }]}, previous=first) self.assertEqual(first["steps"][0]["id"], "implement") self.assertEqual(first["steps"][0]["acceptance_ids"], repeated["steps"][0]["acceptance_ids"]) self.assertEqual(first["steps"][0]["acceptance_ids"], revised["steps"][0]["acceptance_ids"]) + self.assertNotEqual(first["fingerprint"], changed_operation["fingerprint"]) self.assertEqual(first["bounds"]["workspace_paths"], ["docs", "src"]) self.assertEqual(first["bounds"]["risk_ceiling"], "normal") self.assertEqual(len(first["fingerprint"]), 64) @@ -910,7 +916,7 @@ def test_exact_acceptance_phrases_and_control_parser(self): 'PlanProposal\n```json\n{"steps": []}\n```', "PlanProposal", ), {"steps": []}) - def test_provider_shaped_plan_discards_executable_step_metadata(self): + def test_provider_shaped_plan_keeps_only_explicit_approved_file_contracts(self): shaped = json.dumps({ "plan_name": "Status page", "mode": "plan", "do_not_execute_until_approved": True, @@ -918,12 +924,16 @@ def test_provider_shaped_plan_discards_executable_step_metadata(self): "bounds": {"workspace_paths": ["site"], "capabilities": ["read", "write"], "effects": ["read_only", "filesystem"], "risk_ceiling": "normal"}, "steps": [{"step": 1, "title": "Create page", "details": "Write index.html.", - "acceptance_criteria": ["index.html exists"]}], + "acceptance_criteria": ["index.html exists"], + "execution": {"action": "write_file", "arguments": { + "path": "site/index.html", "content": "approved", + }} }], }) parsed = self.agent._observe_model_response(shaped) self.assertEqual(parsed["plan_proposal"]["title"], "Status page") self.assertEqual(parsed["plan_proposal"]["steps"][0]["id"], "step-1") self.assertEqual(parsed["plan_proposal"]["bounds"]["workspace_paths"], ["site"]) + self.assertEqual(parsed["plan_proposal"]["steps"][0]["execution"]["arguments"]["path"], "site/index.html") self.assertEqual(parsed["tool_calls"], []) executable = json.dumps({ @@ -943,6 +953,30 @@ def test_provider_shaped_plan_discards_executable_step_metadata(self): ["Step completed with observable evidence"], ) + def test_strategic_file_execution_contract_is_validated_and_rendered_for_approval(self): + plan = validate_plan_proposal({ + "title": "Create page", "summary": "Write the approved page.", + "bounds": {"workspace_paths": ["site"], "capabilities": ["write"], + "effects": ["filesystem"], "risk_ceiling": "normal"}, + "steps": [{"id": "page", "title": "Create page", "description": "Write site/index.html.", + "acceptance": ["Page exists"], "execution": { + "action": "write_file", "arguments": {"path": "site/index.html", "content": "ok"}, + }}], + }) + self.assertIn('"path": "site/index.html"', render_plan(plan)) + with self.assertRaises(InteractionError): + validate_plan_proposal({**plan, "steps": [{**plan["steps"][0], "execution": { + "action": "run_cmd", "arguments": {"command": "rm -rf ."}, + }}]}) + for execution in ( + {"action": [], "arguments": {"path": "site/index.html", "content": "ok"}}, + {"action": "write_file", "arguments": {"path": "site/index.html"}}, + {"action": "edit_file", "arguments": {"path": "site/index.html", "old_text": "x", + "new_text": "y", "expected_sha256": "stale"}}, + ): + with self.subTest(execution=execution), self.assertRaises(InteractionError): + validate_plan_proposal({**plan, "steps": [{**plan["steps"][0], "execution": execution}]}) + def test_plan_mode_converts_plain_plan_and_discards_early_actions(self): mode_token = self.agent._active_interaction_mode.set("plan") previous_controller = self.agent._interaction_controller @@ -1058,8 +1092,12 @@ def test_plan_prompt_advertises_only_read_and_network_tools(self): self.assertIn("PlanProposal", prompt) self.assertIn("read_file", prompt) self.assertIn("search_web", prompt) - for forbidden in ("write_file", "run_cmd", "TaskList", "TaskDone", "DefineTool", "git_commit"): + for forbidden in ("TaskList", "TaskDone", "DefineTool", "git_commit"): self.assertNotIn(forbidden, prompt) + readonly_inventory = prompt.split("## Available read-only tools\n", 1)[1].split("\n## ", 1)[0] + self.assertNotIn("- write_file:", readonly_inventory) + self.assertNotIn("- run_cmd:", readonly_inventory) + self.assertIn("does not execute before acceptance", prompt) self.assertIn("The first Plan-mode response must be exactly one listed read-only Action", prompt) self.assertIn('"criterion":"Observable result"', prompt) self.assertIn('"verification":{"action":"read_file"', prompt) From 50a24cb801ab3c2b6a916900ab29d13b022df0be Mon Sep 17 00:00:00 2001 From: Evan Gong Date: Sat, 17 Oct 2026 08:50:37 +0800 Subject: [PATCH 10/16] fix: verify strategic read criteria against file contents --- .../plans/2026-10-16-v3-issue-245.md | 6 ++++ docs/tasks-and-automation.md | 2 +- openkyrozen/agent/modes.py | 17 ++++++++-- openkyrozen/agent/prompts.py | 8 ++--- openkyrozen/tasks/engine.py | 33 +++++++++++++++--- tests/test_execution_receipts.py | 1 + tests/test_interaction.py | 34 +++++++++++++++---- 7 files changed, 83 insertions(+), 18 deletions(-) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index ca6efbf..397eed8 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -142,3 +142,9 @@ The update remains limited to #245. Leave PR #285 and issue #245 open and unmerg - PASS: interaction, Working Plan, receipt, task-consistency, and delegation suites — 213 tests on Python 3.12; `make check`, `make lint`, and `make docs-check` pass. - BLOCKED: local Python 3.13 focused suite could not be provisioned because PyPI DNS/network access is unavailable; CI will provide supported-runtime validation. - [ ] Rerun final-head CI and obtain independent automated review before beginning #246. + +### Read-verifier evidence review finding + +- [x] Reproduced the finding by tracing `read_file` receipts with `not_applicable` verification into criterion-ID reconciliation: action/path equality alone credited arbitrary content claims. +- [x] Require an explicit `equals` or `contains` predicate for Strategic Plan `read_file` verifiers, display and fingerprint it, and compare it with the actual returned text before recording criterion evidence. +- [x] Add wrong-content and matching-content regressions, update existing read-verifier fixtures/docs, and rerun focused suites; final-head CI remains pending. diff --git a/docs/tasks-and-automation.md b/docs/tasks-and-automation.md index 5e15e18..425c636 100644 --- a/docs/tasks-and-automation.md +++ b/docs/tasks-and-automation.md @@ -13,7 +13,7 @@ Task IDs and records are actor/workspace/session scoped. The web task API expose Simple work does not require a plan, and a plan is optional for medium work. Complex direct work uses a durable Working Plan with runtime-owned stable IDs and explicit acceptance criteria. The typed `update_plan` boundary applies revision-checked batches of add, update, cancel, reorder, and dependency operations; legacy `complexity`/`tasks` input is adapted through the same validation path. A changed task must still be pending and unexecuted; executed task records and evidence are preserved, and changed work is represented by a new task after cancellation. Scheduling follows plan order and waits for every dependency to succeed. A plan completes only when every task succeeds with acceptance evidence; recovery also reconciles a crash between the final task write and the plan status write. Clearing an active plan cancels its unfinished steps. The existing foreground text protocol continues to use Plan/TaskList compatibility until the native foreground loop is migrated. -Plan Mode can produce a versioned Strategic Plan before execution. Acceptance records the exact objectives, acceptance criteria, workspace paths, capabilities, effect categories, and risk ceiling alongside its linked Working Plan. A step may also carry a visible, fingerprinted `execution` contract for one exact `write_file` or `edit_file` call; delegated file mutations require this explicit contract and still pass assigned-file, workspace-bound, and tool-approval checks. Each criterion should include a `verification` action and exact object arguments; only a successful receipt matching that contract records the criterion ID as verified. Plain-text criteria remain readable for legacy proposals but cannot satisfy the per-criterion completion gate without a verifier. Runtime tool dispatch checks each tool's catalog policy and supplied paths against those accepted bounds; shell processes are denied because this runtime cannot confine arbitrary commands to those paths. An unbounded legacy proposal receives read-only defaults. Strategic acceptance does not grant capabilities, bypass tool approvals, or establish completion evidence. +Plan Mode can produce a versioned Strategic Plan before execution. Acceptance records the exact objectives, acceptance criteria, workspace paths, capabilities, effect categories, and risk ceiling alongside its linked Working Plan. A step may also carry a visible, fingerprinted `execution` contract for one exact `write_file` or `edit_file` call; delegated file mutations require this explicit contract and still pass assigned-file, workspace-bound, and tool-approval checks. Each criterion should include a `verification` action and exact object arguments. A `read_file` verifier must also include one case-sensitive `expect` predicate (`equals` or nonempty `contains`); the returned text must satisfy it before the criterion is recorded. Plain-text criteria remain readable for legacy proposals but cannot satisfy the per-criterion completion gate without a verifier. Runtime tool dispatch checks each tool's catalog policy and supplied paths against those accepted bounds; shell processes are denied because this runtime cannot confine arbitrary commands to those paths. An unbounded legacy proposal receives read-only defaults. Strategic acceptance does not grant capabilities, bypass tool approvals, or establish completion evidence. For a typed plan step, include an optional `execution` object with the canonical tool `action` and object `arguments`. Native tool calls associate with a task only when the runtime receives that task's explicit ID; matching action and arguments alone never select a task. A successful associated receipt may add execution evidence, but it cannot satisfy acceptance criteria by itself. A delegated assignment may carry that explicit Working Plan task ID; the parent records task evidence only after the independent review succeeds. The legacy text adapter retains its bounded matching behavior for compatibility. Plan IDs and task IDs are returned as structured result data so later updates can retain runtime-owned task identities. diff --git a/openkyrozen/agent/modes.py b/openkyrozen/agent/modes.py index 687bc62..d9ca7cb 100644 --- a/openkyrozen/agent/modes.py +++ b/openkyrozen/agent/modes.py @@ -222,10 +222,20 @@ def validate_plan_proposal(value: Any, *, previous: dict[str, Any] | None = None if isinstance(item, dict) and set(item) == {"criterion", "verification"}: verification = item["verification"] if verification is not None: - if (not isinstance(verification, dict) or set(verification) != {"action", "arguments"} + if (not isinstance(verification, dict) + or set(verification) not in ({"action", "arguments"}, {"action", "arguments", "expect"}) or not isinstance(verification.get("action"), str) or not verification["action"].strip() or not isinstance(verification.get("arguments"), dict)): raise InteractionError("acceptance verification must contain an action and object arguments") + if verification["action"] == "read_file": + expect = verification.get("expect") + if (not isinstance(expect, dict) or len(expect) != 1 + or set(expect) not in ({"equals"}, {"contains"}) + or not isinstance(next(iter(expect.values())), str) + or ("contains" in expect and not expect["contains"])): + raise InteractionError("read_file verification requires a nonempty equals or contains predicate") + elif "expect" in verification: + raise InteractionError("expected-result predicates are currently supported only for read_file") try: json.dumps(verification["arguments"], ensure_ascii=False, allow_nan=False) except (TypeError, ValueError) as exc: @@ -791,7 +801,10 @@ def render_plan(plan: dict[str, Any]) -> str: f"{json.dumps(step['execution']['arguments'], ensure_ascii=False, sort_keys=True)}") for verification in step["acceptance_verifications"]: if verification: - lines.append(f" Verified by {verification['action']} with the approved arguments.") + detail = f" with the approved arguments" + if verification.get("expect"): + detail += f" and expected {json.dumps(verification['expect'], ensure_ascii=False)}" + lines.append(f" Verified by {verification['action']}{detail}.") else: lines.append(" No verification contract; this criterion cannot pass the completion gate.") bounds = plan["bounds"] diff --git a/openkyrozen/agent/prompts.py b/openkyrozen/agent/prompts.py index 796ff43..5904f68 100644 --- a/openkyrozen/agent/prompts.py +++ b/openkyrozen/agent/prompts.py @@ -153,9 +153,9 @@ def _system_prompt(self, tools_list: str, agent_config: dict[str, Any] | None = "In `ask` mode, answer or investigate with read/network tools only.\n" "In `plan` mode, first use exactly one read-only inspection Action, then output only a PlanProposal block. Do not execute the plan:\n" "PlanProposal:\n```json\n" - '{"title":"Plan title","summary":"Outcome","assumptions":[],"bounds":{"workspace_paths":["src"],"capabilities":["read","write"],"effects":["read_only","filesystem"],"risk_ceiling":"normal"},"steps":[{"id":"objective-1","title":"Step","description":"Work to perform","acceptance":[{"criterion":"File contains the requested text","verification":{"action":"read_file","arguments":{"path":"src/example.txt"}}}]}]}\n' + '{"title":"Plan title","summary":"Outcome","assumptions":[],"bounds":{"workspace_paths":["src"],"capabilities":["read","write"],"effects":["read_only","filesystem"],"risk_ceiling":"normal"},"steps":[{"id":"objective-1","title":"Step","description":"Work to perform","acceptance":[{"criterion":"File contains the requested text","verification":{"action":"read_file","arguments":{"path":"src/example.txt"},"expect":{"contains":"requested text"}}}]}]}\n' "```\n" - "Use 1-10 stable objective IDs and list exact workspace-relative paths, capability/effect categories, and a normal or high risk ceiling in bounds. Give each acceptance criterion a verification action and exact arguments; only matching successful receipts prove that criterion. A revision is a new version created by the runtime.\n" + "Use 1-10 stable objective IDs and list exact workspace-relative paths, capability/effect categories, and a normal or high risk ceiling in bounds. Give each acceptance criterion a verification action and exact arguments; for `read_file`, include one case-sensitive `expect` predicate (`equals` or `contains`). Only matching successful receipts that satisfy the predicate prove that criterion. A revision is a new version created by the runtime.\n" "In `agent` mode, execute within the available capabilities and existing approval policy.\n" "Legacy Plan and TaskList execution applies only in `agent` mode.\n" ) @@ -197,8 +197,8 @@ def _system_prompt(self, tools_list: str, agent_config: dict[str, Any] | None = + ( "The final plan must use this standalone control:\n" "PlanProposal:\n```json\n" - '{"title":"Plan title","summary":"Outcome","assumptions":[],"bounds":{"workspace_paths":["src"],"capabilities":["read","write"],"effects":["read_only","filesystem"],"risk_ceiling":"normal"},"steps":[{"id":"objective-1","title":"Step","description":"Work to perform after acceptance","acceptance":[{"criterion":"Observable result","verification":{"action":"read_file","arguments":{"path":"src/result.txt"}}}],"execution":{"action":"write_file","arguments":{"path":"src/result.txt","content":"approved content"}}}]}\n' - "```\nUse 1-10 stable objective IDs and state the smallest workspace paths, capability/effect categories, and normal or high risk ceiling. Include an optional exact `execution` object only for a planned file write/edit; its action and complete arguments are part of user approval, and it does not execute before acceptance. Do not output task controls or tool calls.\n" + '{"title":"Plan title","summary":"Outcome","assumptions":[],"bounds":{"workspace_paths":["src"],"capabilities":["read","write"],"effects":["read_only","filesystem"],"risk_ceiling":"normal"},"steps":[{"id":"objective-1","title":"Step","description":"Work to perform after acceptance","acceptance":[{"criterion":"Observable result","verification":{"action":"read_file","arguments":{"path":"src/result.txt"},"expect":{"contains":"approved content"}}}],"execution":{"action":"write_file","arguments":{"path":"src/result.txt","content":"approved content"}}}]}\n' + "```\nUse 1-10 stable objective IDs and state the smallest workspace paths, capability/effect categories, and normal or high risk ceiling. Every `read_file` verifier must include one case-sensitive `expect` predicate (`equals` or `contains`). Include an optional exact `execution` object only for a planned file write/edit; its action and complete arguments are part of user approval, and it does not execute before acceptance. Do not output task controls or tool calls.\n" if interaction_mode == "plan" else "" ) + "\n## Available read-only tools\n" + readonly_tools + "\n\n" diff --git a/openkyrozen/tasks/engine.py b/openkyrozen/tasks/engine.py index 2241850..8e317f8 100644 --- a/openkyrozen/tasks/engine.py +++ b/openkyrozen/tasks/engine.py @@ -312,10 +312,22 @@ def resolve(value: Any) -> str: binding["acceptance_ids"], fields["acceptance"], verifications): item = {"id": criterion_id, "criterion": criterion} if verification is not None: - if (not isinstance(verification, dict) or set(verification) != {"action", "arguments"} + if (not isinstance(verification, dict) + or set(verification) not in ({"action", "arguments"}, + {"action", "arguments", "expect"}) or not isinstance(verification.get("action"), str) or not isinstance(verification.get("arguments"), dict)): raise ValueError("strategic acceptance verification is invalid") + if verification["action"] == "read_file": + expect = verification.get("expect") + if (not isinstance(expect, dict) or len(expect) != 1 + or set(expect) not in ({"equals"}, {"contains"}) + or not isinstance(next(iter(expect.values())), str) + or ("contains" in expect and not expect["contains"])): + raise ValueError("strategic read_file verification requires an expected result") + item["expect"] = deepcopy(expect) + elif "expect" in verification: + raise ValueError("expected-result predicates are supported only for read_file") item["action"] = _receipt_action(verification["action"]) item["args"] = json.dumps(verification["arguments"], ensure_ascii=False, allow_nan=False, sort_keys=True, separators=(",", ":")) @@ -715,10 +727,21 @@ def record_evidence(self, *, task_id: str | None = None, action: str, result: st for task in targets: candidate = deepcopy(task) if success and verification_status in {"verified", "not_applicable"}: - verified_ids = [criterion["id"] for criterion in candidate.get("acceptance", []) - if isinstance(criterion, dict) and criterion.get("id") - and criterion.get("action") == canonical - and criterion.get("args") == normalized_args] + verified_ids = [] + for criterion in candidate.get("acceptance", []): + if (not isinstance(criterion, dict) or not criterion.get("id") + or criterion.get("action") != canonical or criterion.get("args") != normalized_args): + continue + expect = criterion.get("expect") + if canonical == "read_file": + if not isinstance(expect, dict) or len(expect) != 1: + continue + operator, expected = next(iter(expect.items())) + content = str(result) + if (operator == "equals" and content != expected) or ( + operator == "contains" and expected not in content): + continue + verified_ids.append(criterion["id"]) if verified_ids: item["acceptance_ids"] = verified_ids item.setdefault("acceptance", "verified strategic criterion") diff --git a/tests/test_execution_receipts.py b/tests/test_execution_receipts.py index f0299f6..2bb5ba4 100644 --- a/tests/test_execution_receipts.py +++ b/tests/test_execution_receipts.py @@ -365,6 +365,7 @@ def verified(receipt): task["acceptance"] = [{ "id": criterion_id, "criterion": criterion_id, "action": action, "args": json.dumps(arguments, ensure_ascii=False, sort_keys=True, separators=(",", ":")), + **({"expect": {"contains": "verified"}} if action == "read_file" else {}), } for criterion_id, action, arguments in criteria] task.setdefault("checkpoint", {})["strategic_plan"] = { "acceptance_ids": [criterion_id for criterion_id, _, _ in criteria], diff --git a/tests/test_interaction.py b/tests/test_interaction.py index 59cc589..9303ef6 100644 --- a/tests/test_interaction.py +++ b/tests/test_interaction.py @@ -141,6 +141,16 @@ def test_opaque_shell_process_is_denied_under_path_scoped_plan(self): class StrategicPlanContractTests(unittest.TestCase): + def test_read_file_acceptance_requires_an_expected_content_predicate(self): + with self.assertRaises(InteractionError): + validate_plan_proposal({ + "title": "Change", "summary": "Make the change.", + "steps": [{"id": "inspect", "title": "Inspect", "description": "Read the file.", + "acceptance": [{"criterion": "File contains READY", "verification": { + "action": "read_file", "arguments": {"path": "src/file.txt"}, + }}]}], + }) + def test_revalidating_a_normalized_plan_preserves_verified_acceptance_contracts(self): proposal = { "title": "Change", "summary": "Make the change.", @@ -148,6 +158,7 @@ def test_revalidating_a_normalized_plan_preserves_verified_acceptance_contracts( "execution": {"action": "write_file", "arguments": {"path": "src/file.txt", "content": "x"}}, "acceptance": [{"criterion": "File is readable", "verification": { "action": "read_file", "arguments": {"path": "src/file.txt"}, + "expect": {"contains": "x"}, }}]}], } first = validate_plan_proposal(proposal) @@ -320,6 +331,7 @@ def test_legacy_receipt_matching_preserves_strategic_task_binding(self): "steps": [{"id": "read", "title": "Read", "description": "Read src/page.txt.", "acceptance": [{"criterion": "Page is readable", "verification": { "action": "read_file", "arguments": {"path": "src/page.txt"}, + "expect": {"contains": "contents"}, }}]}], }) self.controller.accept_plan(self.manager, plan_id=plan["plan_id"], version=plan["version"]) @@ -547,7 +559,8 @@ def test_real_receipts_cover_only_their_matching_strategic_criteria(self): {"criterion": "Page exists", "verification": { "action": "write_file", "arguments": {"path": "site/index.html", "content": "ok"}}}, {"criterion": "Page can be read back", "verification": { - "action": "read_file", "arguments": {"path": "site/index.html"}}}, + "action": "read_file", "arguments": {"path": "site/index.html"}, + "expect": {"contains": "ok"}}}, ]}], }) accepted, _ = self.controller.accept_plan( @@ -569,6 +582,10 @@ def test_real_receipts_cover_only_their_matching_strategic_criteria(self): result="file written", success=True, verification_status="verified") self.assertEqual(first["acceptance_ids"], [task["acceptance"][0]["id"]]) self.assertNotIn(task["acceptance"][1]["id"], first["acceptance_ids"]) + wrong_read = self.manager.record_evidence(task_id=task["id"], action="read_file", + args='{"path":"site/index.html"}', result="wrong", success=True, + verification_status="not_applicable") + self.assertNotIn("acceptance_ids", wrong_read) second = self.manager.record_evidence(task_id=task["id"], action="read_file", args='{"path":"site/index.html"}', result="ok", success=True, verification_status="not_applicable") @@ -592,7 +609,7 @@ def test_runtime_receipts_keep_multi_criterion_task_active_until_all_are_verifie {"criterion": "Page exists", "verification": {"action": "write_file", "arguments": {"path": "site/index.html", "content": large_content}}}, {"criterion": "Page can be read back", "verification": {"action": "read_file", - "arguments": {"path": "site/index.html"}}}, + "arguments": {"path": "site/index.html"}, "expect": {"equals": large_content}}}, ]}], }) self.controller.accept_plan(self.manager, plan_id=proposal["plan_id"], version=proposal["version"]) @@ -613,9 +630,11 @@ def test_runtime_receipts_keep_multi_criterion_task_active_until_all_are_verifie args_sha256=hashlib.sha256(json.dumps( arguments, ensure_ascii=False, sort_keys=True, separators=(",", ":") ).encode()).hexdigest(), - authorized=True, started_at="start", completed_at="end", success=True, result="ok", + authorized=True, started_at="start", completed_at="end", success=True, + result=large_content if action == "read_file" else "ok", call_id=f"call-{index}", - call_id_source="native", arguments=arguments, tool_result=ToolResult(text="ok"), + call_id_source="native", arguments=arguments, + tool_result=ToolResult(text=large_content if action == "read_file" else "ok"), user_id="local", workspace_id="project", session_id="strategic-session", plan_id=self.manager.working_plan.id, task_id=task["id"], side_effects="filesystem", verification={"status": "verified", "method": "fixture", "observed": None}, @@ -1101,6 +1120,7 @@ def test_plan_prompt_advertises_only_read_and_network_tools(self): self.assertIn("The first Plan-mode response must be exactly one listed read-only Action", prompt) self.assertIn('"criterion":"Observable result"', prompt) self.assertIn('"verification":{"action":"read_file"', prompt) + self.assertIn('"expect":{"contains":"approved content"}', prompt) def test_bounded_write_objective_allows_mutation_with_a_separate_read_verifier(self): from openkyrozen.agent.types import ExecutionReceipt @@ -1116,6 +1136,7 @@ def test_bounded_write_objective_allows_mutation_with_a_separate_read_verifier(s }, "steps": [{"id": "page", "title": "Write page", "description": "Write the page.", "acceptance": [{"criterion": "Page can be read", "verification": { "action": "read_file", "arguments": {"path": "site/index.html"}, + "expect": {"contains": "contents"}, }}]}], }) controller.accept_plan(manager, plan_id=proposal["plan_id"], version=proposal["version"]) @@ -1421,6 +1442,7 @@ def test_accepted_plan_rejects_unrelated_mutation_before_execution(self): "description": "Write index.html with the requested markup.", "acceptance": [{"criterion": "index.html exists", "verification": { "action": "read_file", "arguments": {"path": "index.html"}, + "expect": {"contains": " Date: Sat, 17 Oct 2026 08:54:18 +0800 Subject: [PATCH 11/16] test: add predicates to strategic workflow acceptance --- docs/superpowers/plans/2026-10-16-v3-issue-245.md | 1 + scripts/agent_workflow_acceptance.py | 2 ++ 2 files changed, 3 insertions(+) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index 397eed8..dce2ff2 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -148,3 +148,4 @@ The update remains limited to #245. Leave PR #285 and issue #245 open and unmerg - [x] Reproduced the finding by tracing `read_file` receipts with `not_applicable` verification into criterion-ID reconciliation: action/path equality alone credited arbitrary content claims. - [x] Require an explicit `equals` or `contains` predicate for Strategic Plan `read_file` verifiers, display and fingerprint it, and compare it with the actual returned text before recording criterion evidence. - [x] Add wrong-content and matching-content regressions, update existing read-verifier fixtures/docs, and rerun focused suites; final-head CI remains pending. +- [x] Update the installed-runtime workflow acceptance fixture with read predicates; its loopback acceptance now passes locally. diff --git a/scripts/agent_workflow_acceptance.py b/scripts/agent_workflow_acceptance.py index eff9de4..c9f134c 100644 --- a/scripts/agent_workflow_acceptance.py +++ b/scripts/agent_workflow_acceptance.py @@ -105,6 +105,7 @@ def test_page_is_served(self): {"step_id": "edit", "title": "Edit index.html safely", "details": "Update the draft page before verification.", "acceptance_criteria": [{"criterion": "index.html contains verified", "verification": { "action": "read_file", "arguments": {"path": "index.html"}, + "expect": {"contains": "verified"}, }}]}, {"step_id": "test", "title": "Create test_deploy.py", "details": "Write test_deploy.py.", "acceptance_criteria": [{"criterion": "test_deploy.py exists", "verification": { @@ -114,6 +115,7 @@ def test_page_is_served(self): "details": "Read the deployment test; the acceptance harness runs it after the plan finishes.", "acceptance_criteria": [{"criterion": "unittest exits successfully", "verification": { "action": "read_file", "arguments": {"path": "test_deploy.py"}, + "expect": {"contains": "unittest"}, }}]}, ], } From 1ba8bb8f5a828485b3d9d60b7ec6bfa33043e81c Mon Sep 17 00:00:00 2001 From: Evan Gong Date: Sat, 17 Oct 2026 09:00:26 +0800 Subject: [PATCH 12/16] fix: keep plan guidance within compact prompt budget --- docs/superpowers/plans/2026-10-16-v3-issue-245.md | 1 + openkyrozen/agent/prompts.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index dce2ff2..31bf312 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -149,3 +149,4 @@ The update remains limited to #245. Leave PR #285 and issue #245 open and unmerg - [x] Require an explicit `equals` or `contains` predicate for Strategic Plan `read_file` verifiers, display and fingerprint it, and compare it with the actual returned text before recording criterion evidence. - [x] Add wrong-content and matching-content regressions, update existing read-verifier fixtures/docs, and rerun focused suites; final-head CI remains pending. - [x] Update the installed-runtime workflow acceptance fixture with read predicates; its loopback acceptance now passes locally. +- [x] Tighten the shared predicate guidance after full CI caught compact-prompt length drift; both compact-size and Plan-mode prompt regressions pass locally. diff --git a/openkyrozen/agent/prompts.py b/openkyrozen/agent/prompts.py index 5904f68..573ab67 100644 --- a/openkyrozen/agent/prompts.py +++ b/openkyrozen/agent/prompts.py @@ -155,7 +155,7 @@ def _system_prompt(self, tools_list: str, agent_config: dict[str, Any] | None = "PlanProposal:\n```json\n" '{"title":"Plan title","summary":"Outcome","assumptions":[],"bounds":{"workspace_paths":["src"],"capabilities":["read","write"],"effects":["read_only","filesystem"],"risk_ceiling":"normal"},"steps":[{"id":"objective-1","title":"Step","description":"Work to perform","acceptance":[{"criterion":"File contains the requested text","verification":{"action":"read_file","arguments":{"path":"src/example.txt"},"expect":{"contains":"requested text"}}}]}]}\n' "```\n" - "Use 1-10 stable objective IDs and list exact workspace-relative paths, capability/effect categories, and a normal or high risk ceiling in bounds. Give each acceptance criterion a verification action and exact arguments; for `read_file`, include one case-sensitive `expect` predicate (`equals` or `contains`). Only matching successful receipts that satisfy the predicate prove that criterion. A revision is a new version created by the runtime.\n" + "Use 1-10 stable objective IDs and exact workspace paths, capabilities/effects, and risk ceiling. Give each criterion a verification action and exact arguments. For `read_file`, add one case-sensitive `expect` (`equals` or nonempty `contains`); only matching output verifies it.\n" "In `agent` mode, execute within the available capabilities and existing approval policy.\n" "Legacy Plan and TaskList execution applies only in `agent` mode.\n" ) From 690fd18af1e402845339e55a907878a5ac6bb1dc Mon Sep 17 00:00:00 2001 From: Evan Gong Date: Sat, 17 Oct 2026 09:11:36 +0800 Subject: [PATCH 13/16] fix: close strategic plan review gaps --- .../plans/2026-10-16-v3-issue-245.md | 11 +++ openkyrozen/agent/delegation_runtime.py | 9 +- openkyrozen/agent/executor.py | 15 ++- openkyrozen/agent/modes.py | 16 +++- openkyrozen/tasks/engine.py | 5 +- tests/test_delegation.py | 37 +++++++ tests/test_interaction.py | 96 ++++++++++++++++++- 7 files changed, 173 insertions(+), 16 deletions(-) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index 31bf312..32dd621 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -150,3 +150,14 @@ The update remains limited to #245. Leave PR #285 and issue #245 open and unmerg - [x] Add wrong-content and matching-content regressions, update existing read-verifier fixtures/docs, and rerun focused suites; final-head CI remains pending. - [x] Update the installed-runtime workflow acceptance fixture with read predicates; its loopback acceptance now passes locally. - [x] Tighten the shared predicate guidance after full CI caught compact-prompt length drift; both compact-size and Plan-mode prompt regressions pass locally. + +### Final-head Codex review findings + +- [x] The workflow-verifier predicate finding was already fixed in `b8bc73f`; the review was against its parent head, and loopback acceptance confirms the fixture now supplies both predicates. +- [x] Exclude cancelled tasks from prior Strategic Plan revisions when validating completion; retain completion checks for every noncancelled member and require its current strategic binding. +- [x] Verify `read_file` predicates against the private, full `ToolResult.text`, while keeping persisted evidence bounded and redacted; regressions cover content beyond the public projection limit and redacted projection text. +- [x] Require exact checkpoint matching whenever a task has an explicit `execution` contract; retain bounded path-based mutation support only for tasks without an exact contract. +- [x] Apply parent Strategic Plan capability/effect/path bounds to every delegated action, including reads; retain task matching for mutations. +- [x] Reject Strategic Plan acceptance while an unrelated active Working Plan exists, preserving that plan and its tasks. +- [x] Derive new proposal versions from the latest historical proposal for the executing plan, including cancelled revisions, so stale approval identifiers cannot target a replacement. +- [ ] Run final focused/full CI, refresh automated review, and resolve all remaining review threads before continuing to #246. diff --git a/openkyrozen/agent/delegation_runtime.py b/openkyrozen/agent/delegation_runtime.py index b022d9c..a13980d 100644 --- a/openkyrozen/agent/delegation_runtime.py +++ b/openkyrozen/agent/delegation_runtime.py @@ -362,7 +362,7 @@ def _delegation_tool_access(self, action, args): raise ValueError("Sub-agent write is outside its assigned files") parent = getattr(self.current_session, "_delegation_parent", None) parent_plan = parent.interaction.state().get("executing_plan") if parent else None - if parent and parent_plan and self._is_state_changing_action(action, args): + if parent and parent_plan: from openkyrozen.agent.executor import _legacy_object_arguments, _strategic_plan_denial arguments = args if isinstance(args, dict) else None spec = getattr(self.tool_registry, "specs", {}).get(action) @@ -378,9 +378,10 @@ def _delegation_tool_access(self, action, args): ) if denial: raise PermissionError(f"Sub-agent operation exceeds the accepted Strategic Plan: {denial}") - allowed, reason = parent.tasks.mutation_matches_current_task(action, match_args) - if not allowed: - raise ValueError("Sub-agent action does not match accepted plan: " + reason) + if self._is_state_changing_action(action, args): + allowed, reason = parent.tasks.mutation_matches_current_task(action, match_args) + if not allowed: + raise ValueError("Sub-agent action does not match accepted plan: " + reason) guarded = action in {"read_file", "write_file", "edit_file", "search_files"} or self._is_state_changing_action(action, args) or action in {"git_diff", "git_show"} if access and guarded: cancelled = (lambda: coordinator.cancelled[context.child_run_id].is_set()) if coordinator else (lambda: False) diff --git a/openkyrozen/agent/executor.py b/openkyrozen/agent/executor.py index 7b28eda..eec7bfc 100644 --- a/openkyrozen/agent/executor.py +++ b/openkyrozen/agent/executor.py @@ -515,12 +515,15 @@ def emit_rejected_call(): if self._is_state_changing_action(canonical, args) and active_plan: match_args = (json.dumps(structured, ensure_ascii=False, sort_keys=True, separators=(",", ":")) if isinstance(structured, dict) and structured else args) + current_task = next((task for task in self.tasks.tasks + if self.tasks.working_plan and task.get("id") in self.tasks.working_plan.task_ids + and task.get("status") in {"pending", "running"} + and self.tasks._dependencies_succeeded(task, self.tasks.tasks)), {}) + checkpoint = current_task.get("checkpoint") or {} scoped_file_mutation = ( - canonical in {"write_file", "edit_file"} - and _file_mutation_matches_objective(next((task for task in self.tasks.tasks - if self.tasks.working_plan and task.get("id") in self.tasks.working_plan.task_ids - and task.get("status") in {"pending", "running"} - and self.tasks._dependencies_succeeded(task, self.tasks.tasks)), {}), structured) + not checkpoint.get("action") + and canonical in {"write_file", "edit_file"} + and _file_mutation_matches_objective(current_task, structured) and _strategic_plan_denial(active_plan, canonical, structured, expected_spec, self._get_workspace_root()) is None ) @@ -602,6 +605,8 @@ def _record_turn_receipt(self, receipt: ExecutionReceipt, *, allow_legacy_task_m result=receipt_projection["result"], success=receipt.success, acceptance=task_acceptance, receipt_id=receipt.receipt_id, verification_status=receipt.verification.get("status"), + verification_result=(receipt.tool_result.text if receipt.action == "read_file" + and receipt.tool_result is not None else None), ) task_id = evidence.get("task_id") effective_acceptance = evidence.get("acceptance") or ( diff --git a/openkyrozen/agent/modes.py b/openkyrozen/agent/modes.py index d9ca7cb..7814614 100644 --- a/openkyrozen/agent/modes.py +++ b/openkyrozen/agent/modes.py @@ -598,6 +598,12 @@ def reopen_question(self) -> dict[str, Any]: def propose_plan(self, value: Any, *, original_input: str = "") -> dict[str, Any]: state = self.state() previous = state.get("pending_plan") or state.get("executing_plan") + if previous and state.get("pending_plan") is None: + latest = [int(event.get("payload", {}).get("version", 0)) for event in self._history() + if event.get("event_type") == "interaction.plan_proposed" + and event.get("payload", {}).get("plan_id") == previous.get("plan_id")] + if latest: + previous = {**previous, "version": max(latest)} plan = validate_plan_proposal(value, previous=previous, original_input=original_input) self._append("interaction.plan_proposed", plan) return plan @@ -645,6 +651,11 @@ def accept_plan(self, task_manager: Any, *, plan_id: str | None = None, raise InteractionError("plan is not pending") if executing and task_manager.working_plan is None: task_manager.recover() + elif not executing and task_manager.working_plan is None: + task_manager.recover() + if (not executing and task_manager.working_plan is not None + and task_manager.working_plan.status == "active"): + raise InteractionError("cannot accept a Strategic Plan while an unrelated Working Plan is active") working_plan = task_manager.working_plan if executing else None previous_tasks = [task for task in task_manager.tasks if working_plan and task.get("id") in working_plan.task_ids] @@ -728,7 +739,10 @@ def complete_plan(self, plan: dict[str, Any], *, status: str = "completed", for previous in prior_completions): return working = task_manager.working_plan if task_manager is not None else None - task_ids = (list(working.task_ids) if working and working.id == plan.get("working_plan_id") + current_tasks = {task.get("id"): task for task in task_manager.tasks} if task_manager is not None else {} + task_ids = ([task_id for task_id in working.task_ids + if current_tasks.get(task_id, {}).get("status") != "cancelled"] + if working and working.id == plan.get("working_plan_id") else plan.get("working_task_ids")) if status == "completed": if task_manager is None or not isinstance(task_ids, list) or not task_ids: diff --git a/openkyrozen/tasks/engine.py b/openkyrozen/tasks/engine.py index 8e317f8..25f0df8 100644 --- a/openkyrozen/tasks/engine.py +++ b/openkyrozen/tasks/engine.py @@ -684,7 +684,8 @@ def record_evidence(self, *, task_id: str | None = None, action: str, result: st acceptance: str | None = None, args: str | None = None, args_projection: str | None = None, args_fingerprint: str | None = None, receipt_id: str | None = None, - verification_status: str | None = None) -> dict[str, Any]: + verification_status: str | None = None, + verification_result: str | None = None) -> dict[str, Any]: canonical = _receipt_action(action) normalized_args = _receipt_args(args) persisted_args = _receipt_args(args_projection) if args_projection is not None else normalized_args @@ -737,7 +738,7 @@ def record_evidence(self, *, task_id: str | None = None, action: str, result: st if not isinstance(expect, dict) or len(expect) != 1: continue operator, expected = next(iter(expect.items())) - content = str(result) + content = str(verification_result if verification_result is not None else result) if (operator == "equals" and content != expected) or ( operator == "contains" and expected not in content): continue diff --git a/tests/test_delegation.py b/tests/test_delegation.py index 4976887..f4e7e1c 100644 --- a/tests/test_delegation.py +++ b/tests/test_delegation.py @@ -165,6 +165,43 @@ def test_child_can_use_read_only_network_tools(self): finally: self.runtime._turn_context.reset(context_token) + def test_parent_strategic_bounds_restrict_delegated_reads(self): + from openkyrozen.agent.turn import ExecutionContext + from types import SimpleNamespace + + outside = self.root / "outside.txt" + outside.write_text("private", encoding="utf-8") + parent = SimpleNamespace( + interaction=SimpleNamespace(state=lambda: {"executing_plan": { + "bounds": {"workspace_paths": ["allowed"], "capabilities": ["read"], + "effects": ["read_only"], "risk_ceiling": "normal"}, + }}), + workspace=SimpleNamespace(root=self.root), + tasks=SimpleNamespace(), + ) + session = self.runtime.current_session + previous_parent = getattr(session, "_delegation_parent", None) + session._delegation_parent = parent + child_id = "out-of-plan-reader" + coordinator = SimpleNamespace( + access=WorkspaceAccess(), root=self.root.resolve(), + runs={child_id: {"assignment": brief(scope=["outside.txt"])}}, + cancelled={child_id: threading.Event()}, check_cancelled=lambda _run_id: None, + ) + token = self.runtime._turn_context.set( + ExecutionContext(child_run_id=child_id, coordinator=coordinator), + ) + try: + with self.assertRaisesRegex(PermissionError, "accepted Strategic Plan"): + with self.runtime._delegation_tool_access("read_file", "outside.txt"): + self.fail("child read outside parent Strategic Plan bounds must not run") + finally: + self.runtime._turn_context.reset(token) + if previous_parent is None: + del session._delegation_parent + else: + session._delegation_parent = previous_parent + def manager(self, invoke, concurrency=4): coordinator = Delegation(self.runtime.memory_bank, invoke, profiles=self.runtime.subagent_manager.profiles, root=self.root, concurrency=concurrency) diff --git a/tests/test_interaction.py b/tests/test_interaction.py index 9303ef6..468ff7d 100644 --- a/tests/test_interaction.py +++ b/tests/test_interaction.py @@ -303,6 +303,39 @@ def test_accepting_material_revision_replaces_executing_plan_atomically(self): self.assertEqual(len(updated["working_task_ids"]), 1) self.assertNotEqual(updated["working_task_ids"][0], old_task_id) + def test_revised_plan_completion_ignores_cancelled_tasks_from_previous_versions(self): + first = self._proposal() + self.controller.accept_plan(self.manager, plan_id=first["plan_id"], version=first["version"]) + revised = self.controller.propose_plan({ + "title": "Revised page", "summary": "Update the accepted page objective.", + "bounds": first["bounds"], + "steps": [{"id": "page", "title": "Create revised page", "description": "Write the revised page.", + "acceptance": first["steps"][0]["acceptance"]}], + }) + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=revised["plan_id"], version=revised["version"], + ) + task = next(task for task in self.manager.tasks if task["id"] == accepted["working_task_ids"][0]) + evidence = {"success": True, "acceptance": "verified", + "acceptance_ids": task["checkpoint"]["strategic_plan"]["acceptance_ids"]} + self.assertTrue(self.manager.set_status(self.manager.tasks.index(task), "succeeded", evidence=evidence)) + self.controller.complete_plan(accepted, task_manager=self.manager) + self.assertIsNone(self.controller.state()["executing_plan"]) + + def test_active_unrelated_working_plan_is_preserved_when_strategic_acceptance_is_rejected(self): + self.manager.update_plan(complexity="complex", tasks=[{ + "description": "Existing work", "acceptance": ["Existing work is done"], + }]) + existing_id = self.manager.tasks[0]["id"] + plan = self._proposal() + with self.assertRaises(InteractionError): + self.controller.accept_plan(self.manager, plan_id=plan["plan_id"], version=plan["version"]) + self.assertEqual(self.manager.tasks[0]["id"], existing_id) + self.assertEqual(self.manager.tasks[0]["description"], "Existing work") + self.assertEqual(self.store.list_events( + "interaction.plan_accepted", workspace_id="project", session_id="strategic-session", + ), []) + def test_restart_reconciles_completion_after_working_plan_terminalization(self): plan = self._proposal() accepted, _ = self.controller.accept_plan( @@ -499,6 +532,23 @@ def test_staging_and_cancelling_revision_preserves_accepted_plan_bounds(self): self.controller.cancel_plan(revision["plan_id"], revision["version"]) self.assertEqual(self.controller.state()["executing_plan"]["fingerprint"], accepted["fingerprint"]) + def test_cancelled_revision_numbers_are_not_reused(self): + plan = self._proposal() + accepted, _ = self.controller.accept_plan( + self.manager, plan_id=plan["plan_id"], version=plan["version"], + ) + revised = self.controller.propose_plan({ + "title": "First revision", "summary": "Stage revision one.", + "bounds": plan["bounds"], "steps": plan["steps"], + }) + self.controller.cancel_plan(revised["plan_id"], revised["version"]) + replacement = self.controller.propose_plan({ + "title": "Replacement revision", "summary": "Stage a distinct revision.", + "bounds": plan["bounds"], "steps": plan["steps"], + }) + self.assertEqual(replacement["plan_id"], accepted["plan_id"]) + self.assertEqual(replacement["version"], revised["version"] + 1) + def test_acceptance_rejects_proposal_superseded_before_transaction(self): plan = self._proposal() edit_plan = self.manager.edit_plan @@ -587,7 +637,8 @@ def test_real_receipts_cover_only_their_matching_strategic_criteria(self): verification_status="not_applicable") self.assertNotIn("acceptance_ids", wrong_read) second = self.manager.record_evidence(task_id=task["id"], action="read_file", - args='{"path":"site/index.html"}', result="ok", success=True, + args='{"path":"site/index.html"}', result="token=", + verification_result="token=ok", success=True, verification_status="not_applicable") self.assertEqual(second["acceptance_ids"], [task["acceptance"][1]["id"]]) self.manager.set_status(0, "succeeded") @@ -599,7 +650,7 @@ def test_runtime_receipts_keep_multi_criterion_task_active_until_all_are_verifie from types import SimpleNamespace import hashlib - large_content = "x" * 1500 + large_content = "x " * 100_000 + "UNIQUE-END" proposal = self.controller.propose_plan({ "title": "Verified page", "summary": "Create and inspect a page.", "bounds": {"workspace_paths": ["site"], "capabilities": ["read", "write"], @@ -609,7 +660,7 @@ def test_runtime_receipts_keep_multi_criterion_task_active_until_all_are_verifie {"criterion": "Page exists", "verification": {"action": "write_file", "arguments": {"path": "site/index.html", "content": large_content}}}, {"criterion": "Page can be read back", "verification": {"action": "read_file", - "arguments": {"path": "site/index.html"}, "expect": {"equals": large_content}}}, + "arguments": {"path": "site/index.html"}, "expect": {"contains": "UNIQUE-END"}}}, ]}], }) self.controller.accept_plan(self.manager, plan_id=proposal["plan_id"], version=proposal["version"]) @@ -631,7 +682,7 @@ def test_runtime_receipts_keep_multi_criterion_task_active_until_all_are_verifie arguments, ensure_ascii=False, sort_keys=True, separators=(",", ":") ).encode()).hexdigest(), authorized=True, started_at="start", completed_at="end", success=True, - result=large_content if action == "read_file" else "ok", + result=large_content[:200_000] if action == "read_file" else "ok", call_id=f"call-{index}", call_id_source="native", arguments=arguments, tool_result=ToolResult(text=large_content if action == "read_file" else "ok"), @@ -1161,6 +1212,43 @@ def test_bounded_write_objective_allows_mutation_with_a_separate_read_verifier(s self.agent.tasks = previous_tasks self.agent._set_workspace_root(previous_root) + def test_explicit_execution_contract_rejects_changed_write_arguments(self): + from openkyrozen.agent.types import ExecutionReceipt + previous = self.agent._interaction_controller + previous_tasks = self.agent.tasks + manager = TaskManager(self.store, workspace_id="workspace", session_id="session") + controller = InteractionController(self.store, workspace_id="workspace", session_id="session") + approved = {"path": "site/index.html", "content": "approved"} + proposal = controller.propose_plan({ + "title": "Write page", "summary": "Write the approved content.", "bounds": { + "workspace_paths": ["site"], "capabilities": ["write"], + "effects": ["filesystem", "read_only"], "risk_ceiling": "normal", + }, "steps": [{"id": "page", "title": "Write page", "description": "Write the page.", + "execution": {"action": "write_file", "arguments": approved}, + "acceptance": [{"criterion": "Page can be read", "verification": { + "action": "read_file", "arguments": {"path": "site/index.html"}, + "expect": {"contains": "approved"}, + }}]}], + }) + controller.accept_plan(manager, plan_id=proposal["plan_id"], version=proposal["version"]) + self.agent.tasks = manager + self.agent._interaction_controller = controller + receipt = ExecutionReceipt( + "receipt", "operation", "write_file", "site/index.html|different", True, + "start", "end", True, "written", call_id="call", call_id_source="synthetic", + arguments={"path": "site/index.html", "content": "different"}, tool_result=ToolResult(text="written"), + ) + try: + with patch.object(self.agent, "_run_tool", return_value=receipt) as execute: + outcome = self.agent._execute_turn_action( + "write_file", "site/index.html|different", operation_scope="test", successful_operations=set(), + ) + self.assertFalse(outcome.success) + execute.assert_not_called() + finally: + self.agent._interaction_controller = previous + self.agent.tasks = previous_tasks + def test_structured_legacy_arguments_match_the_accepted_action_contract(self): from openkyrozen.agent.types import ExecutionReceipt previous = self.agent._interaction_controller From e9e5c68b981140ed894afc247da9c80fc6ce9d62 Mon Sep 17 00:00:00 2001 From: Evan <170803900+EvanProgramming@users.noreply.github.com> Date: Sat, 17 Oct 2026 13:35:43 +0800 Subject: [PATCH 14/16] fix: close strategic plan review gaps --- openkyrozen/agent/executor.py | 5 +- openkyrozen/agent/modes.py | 37 +++++++++++- openkyrozen/tasks/engine.py | 21 +++++-- tests/test_interaction.py | 105 +++++++++++++++++++++++++++++++++- 4 files changed, 154 insertions(+), 14 deletions(-) diff --git a/openkyrozen/agent/executor.py b/openkyrozen/agent/executor.py index eec7bfc..a0e28a9 100644 --- a/openkyrozen/agent/executor.py +++ b/openkyrozen/agent/executor.py @@ -50,8 +50,7 @@ def _file_mutation_matches_objective(task, arguments): if (isinstance(arguments, dict) and str(arguments.get("path", "")).replace("\\", "/").strip("/").casefold() == normalized): return True - if any(normalized == value or normalized.startswith(value.rstrip("/") + "/") - for value in explicit_paths): + if normalized in explicit_paths: return True if explicit_paths: return False @@ -97,6 +96,8 @@ def _strategic_plan_denial(plan, action, args, spec, workspace_root): values = structured.get(key) if isinstance(values, list) and all(isinstance(value, str) and value.strip() for value in values): paths.update({f"{key}:{index}": value for index, value in enumerate(values)}) + if action in {"list_dir", "list_tree", "search_files"} and not paths: + paths = {"path": structured.get("path", ".")} if action == "find_files" and not paths: paths = {"directory": structured.get("directory", ".")} if effect == "filesystem" and not paths: diff --git a/openkyrozen/agent/modes.py b/openkyrozen/agent/modes.py index 7814614..3119cd3 100644 --- a/openkyrozen/agent/modes.py +++ b/openkyrozen/agent/modes.py @@ -324,6 +324,33 @@ def values(field: str, allowed: set[str] | None = None) -> list[str]: "workspace_paths": sorted(normalised_paths), "capabilities": sorted(capabilities), "effects": sorted(effects), "risk_ceiling": risk_ceiling, } + contract_policies = { + "read_file": ("read", "read_only"), + "write_file": ("write", "filesystem"), + "edit_file": ("write", "filesystem"), + } + for step in normalised: + contracts = ([step["execution"]] if step.get("execution") else []) + [ + verification for verification in step["acceptance_verifications"] if verification + ] + for contract in contracts: + policy = contract_policies.get(contract["action"]) + if policy is None: + raise InteractionError(f"unsupported Strategic Plan contract action: {contract['action']}") + capability, effect = policy + if capability not in bounds["capabilities"] or effect not in bounds["effects"]: + raise InteractionError("Strategic Plan contract exceeds its approved capability or effect bounds") + path = contract["arguments"].get("path") + if not isinstance(path, str) or not path.strip(): + raise InteractionError("Strategic Plan file contracts require a path") + normalized_contract_path = posixpath.normpath(path.replace("\\", "/")) + if (normalized_contract_path.startswith("/") or normalized_contract_path == ".." + or normalized_contract_path.startswith("../")): + raise InteractionError("Strategic Plan contract path escapes the approved workspace") + if not any(root == "." or normalized_contract_path == root + or normalized_contract_path.startswith(root.rstrip("/") + "/") + for root in bounds["workspace_paths"]): + raise InteractionError("Strategic Plan contract path is outside its approved workspace bounds") plan = { "plan_id": _text( (previous or {}).get("plan_id") or f"plan_{uuid.uuid4().hex}", @@ -524,6 +551,8 @@ def state(self, user_input: str = "") -> dict[str, Any]: plan = None elif event_type == "interaction.plan_completed": if (executing and payload.get("plan_id") == executing.get("plan_id") + and payload.get("version") == executing.get("version") + and payload.get("fingerprint") == executing.get("fingerprint") and payload.get("status") in {"completed", "cancelled"}): executing = None effective = route_mode( @@ -719,8 +748,9 @@ def accept_plan(self, task_manager: Any, *, plan_id: str | None = None, return accepted, False raise bound_ids = [task["id"] for task in working["tasks"] - if task.get("status") != "cancelled" - and (task.get("checkpoint") or {}).get("strategic_plan", {}).get("plan_id") == plan["plan_id"] + if (task.get("checkpoint") or {}).get("strategic_plan", {}).get("plan_id") == plan["plan_id"] + and (task.get("checkpoint") or {}).get("strategic_plan", {}).get("version") == plan["version"] + and (task.get("checkpoint") or {}).get("strategic_plan", {}).get("fingerprint") == plan["fingerprint"] and (task.get("checkpoint") or {}).get("strategic_plan", {}).get("objective_id") in {step["id"] for step in plan["steps"]}] return {**plan, "working_plan_id": working["plan_id"], @@ -741,7 +771,8 @@ def complete_plan(self, plan: dict[str, Any], *, status: str = "completed", working = task_manager.working_plan if task_manager is not None else None current_tasks = {task.get("id"): task for task in task_manager.tasks} if task_manager is not None else {} task_ids = ([task_id for task_id in working.task_ids - if current_tasks.get(task_id, {}).get("status") != "cancelled"] + if (current_tasks.get(task_id, {}).get("checkpoint") or {}).get("strategic_plan", {}).get("version") == plan.get("version") + and (current_tasks.get(task_id, {}).get("checkpoint") or {}).get("strategic_plan", {}).get("fingerprint") == plan.get("fingerprint")] if working and working.id == plan.get("working_plan_id") else plan.get("working_task_ids")) if status == "completed": diff --git a/openkyrozen/tasks/engine.py b/openkyrozen/tasks/engine.py index 25f0df8..3897067 100644 --- a/openkyrozen/tasks/engine.py +++ b/openkyrozen/tasks/engine.py @@ -549,6 +549,16 @@ def adopt_legacy_plan(self, *, complexity: str = "complex") -> WorkingPlan: def _has_evidence(self, idx: int, evidence: dict[str, Any] | None = None) -> bool: evidence_items = ([evidence] if evidence else []) + self.tasks[idx].get("evidence", []) + strategic = ((self.tasks[idx].get("checkpoint") or {}).get("strategic_plan") or {}) + required = set(strategic.get("acceptance_ids", ())) + if required: + covered = { + criterion_id + for item in evidence_items + if item.get("success") is True + for criterion_id in item.get("acceptance_ids", ()) + } + return required <= covered return any(item.get("success") is True and item.get("acceptance") for item in evidence_items) def _receipt_match_score(self, task: dict[str, Any], action: str, args: str, @@ -559,18 +569,17 @@ def _receipt_match_score(self, task: dict[str, Any], action: str, args: str, checkpoint = task.get("checkpoint") or {} checkpoint_action = _receipt_action(checkpoint.get("action")) checkpoint_args = _receipt_args(checkpoint.get("args")) - if checkpoint_action: - if checkpoint_action != action: - return -1 + if checkpoint_action == action: if checkpoint_args: checkpoint_fingerprint = (checkpoint.get("args_sha256") or hashlib.sha256(checkpoint_args.encode("utf-8")).hexdigest()) if args_fingerprint: if checkpoint_fingerprint != args_fingerprint: - return -1 + checkpoint_action = None elif checkpoint_args != args: - return -1 - return 200 if checkpoint_args else 150 + checkpoint_action = None + if checkpoint_action: + return 200 if checkpoint_args else 150 score = 0 for criterion in task.get("acceptance") or []: diff --git a/tests/test_interaction.py b/tests/test_interaction.py index 468ff7d..7d8f6ab 100644 --- a/tests/test_interaction.py +++ b/tests/test_interaction.py @@ -3,7 +3,11 @@ import unittest from openkyrozen.tools.models import ToolResult from types import SimpleNamespace -from openkyrozen.agent.executor import _legacy_object_arguments, _strategic_plan_denial +from openkyrozen.agent.executor import ( + _file_mutation_matches_objective, + _legacy_object_arguments, + _strategic_plan_denial, +) from pathlib import Path from unittest.mock import patch @@ -61,6 +65,23 @@ def test_full_path_objectives_do_not_match_a_sibling_with_the_same_basename(self self.assertTrue(_file_mutation_matches_objective(scoped_directory, {"path": "src/public/index.html"})) self.assertFalse(_file_mutation_matches_objective(scoped_directory, {"path": "src/private/index.html"})) + def test_pathless_read_defaults_are_checked_against_accepted_scope(self): + with tempfile.TemporaryDirectory() as directory: + plan = {"bounds": {"workspace_paths": ["src"], "capabilities": ["read"], + "effects": ["read_only"], "risk_ceiling": "normal"}} + read = SimpleNamespace(capability="read", side_effects="read", risk="normal") + for action, args in (("list_dir", {}), ("list_tree", ""), ("search_files", {"query": "token"})): + with self.subTest(action=action): + self.assertIn("outside", _strategic_plan_denial( + plan, action, args, read, Path(directory), + )) + + def test_explicit_file_path_does_not_authorize_descendants(self): + objective = {"title": "Write src/public/index.html", "description": "Update the page."} + self.assertFalse(_file_mutation_matches_objective( + objective, {"path": "src/public/index.html/private.txt"}, + )) + def test_legacy_mutation_arguments_are_normalized_for_scope_checks(self): from pathlib import Path with tempfile.TemporaryDirectory() as directory: @@ -119,6 +140,24 @@ def test_legacy_bounds_fail_closed_for_unapproved_effects(self): self.assertIn("capability", _strategic_plan_denial( plan, "write_file", {"path": "file.txt"}, write, Path(directory))) + def test_contract_action_capability_effect_and_path_must_fit_bounds(self): + proposal = {"title": "Build page", "summary": "Write and verify.", "steps": [{ + "id": "page", "title": "Build", "description": "Write src/page.html.", + "execution": {"action": "write_file", "arguments": {"path": "src/page.html", "content": "ok"}}, + "acceptance": [{"criterion": "Page is correct", "verification": { + "action": "read_file", "arguments": {"path": "src/page.html"}, "expect": {"contains": "ok"}, + }}], + }]} + proposal["bounds"] = {"workspace_paths": ["src"], "capabilities": ["write"], + "effects": ["filesystem"], "risk_ceiling": "normal"} + with self.assertRaisesRegex(InteractionError, "capability or effect"): + validate_plan_proposal(proposal) + proposal["bounds"] = {"workspace_paths": ["src"], "capabilities": ["read", "write"], + "effects": ["read_only", "filesystem"], "risk_ceiling": "normal"} + proposal["steps"][0]["acceptance"][0]["verification"]["arguments"]["path"] = "outside/page.html" + with self.assertRaisesRegex(InteractionError, "outside its approved workspace"): + validate_plan_proposal(proposal) + def test_workspace_scope_resolves_symlinks_before_authorizing(self): from pathlib import Path with tempfile.TemporaryDirectory() as directory, tempfile.TemporaryDirectory() as outside: @@ -154,6 +193,8 @@ def test_read_file_acceptance_requires_an_expected_content_predicate(self): def test_revalidating_a_normalized_plan_preserves_verified_acceptance_contracts(self): proposal = { "title": "Change", "summary": "Make the change.", + "bounds": {"workspace_paths": ["src"], "capabilities": ["read", "write"], + "effects": ["read_only", "filesystem"], "risk_ceiling": "normal"}, "steps": [{"id": "inspect", "title": "Inspect", "description": "Read the file.", "execution": {"action": "write_file", "arguments": {"path": "src/file.txt", "content": "x"}}, "acceptance": [{"criterion": "File is readable", "verification": { @@ -322,6 +363,64 @@ def test_revised_plan_completion_ignores_cancelled_tasks_from_previous_versions( self.controller.complete_plan(accepted, task_manager=self.manager) self.assertIsNone(self.controller.state()["executing_plan"]) + def test_cancelled_current_objective_blocks_strategic_completion(self): + plan = self.controller.propose_plan({ + "title": "Build two pages", "summary": "Build and verify both pages.", + "bounds": {"workspace_paths": ["site"], "capabilities": ["read", "write"], + "effects": ["read_only", "filesystem"], "risk_ceiling": "normal"}, + "steps": [{"id": name, "title": f"Build {name}", "description": f"Write site/{name}.html.", + "acceptance": [{"criterion": f"{name} can be read", "verification": { + "action": "read_file", "arguments": {"path": f"site/{name}.html"}, + "expect": {"contains": name}, + }}]} for name in ("first", "second")], + }) + accepted, _ = self.controller.accept_plan(self.manager, plan_id=plan["plan_id"], version=plan["version"]) + first, second = accepted["working_task_ids"] + first_task = next(task for task in self.manager.tasks if task["id"] == first) + self.assertTrue(self.manager.set_status(self.manager.tasks.index(first_task), "cancelled")) + second_task = next(task for task in self.manager.tasks if task["id"] == second) + evidence = {"success": True, "acceptance": "verified", + "acceptance_ids": second_task["checkpoint"]["strategic_plan"]["acceptance_ids"]} + self.assertTrue(self.manager.set_status(self.manager.tasks.index(second_task), "succeeded", evidence=evidence)) + with self.assertRaisesRegex(InteractionError, "acceptance evidence gates"): + self.controller.complete_plan(accepted, task_manager=self.manager) + + def test_delayed_prior_version_completion_does_not_clear_current_plan(self): + first = self._proposal() + old, _ = self.controller.accept_plan(self.manager, plan_id=first["plan_id"], version=first["version"]) + revised = self.controller.propose_plan({ + "title": "Revised page", "summary": "Update the accepted page objective.", + "bounds": first["bounds"], + "steps": [{"id": "page", "title": "Create revised page", "description": "Write the revised page.", + "acceptance": first["steps"][0]["acceptance"]}], + }) + current, _ = self.controller.accept_plan(self.manager, plan_id=revised["plan_id"], version=revised["version"]) + self.store.append_event("interaction.plan_completed", { + "plan_id": old["plan_id"], "version": old["version"], + "fingerprint": old["fingerprint"], "status": "completed", + }, workspace_id="project", session_id="strategic-session") + state = self.controller.state()["executing_plan"] + self.assertEqual(state["version"], current["version"]) + self.assertEqual(state["fingerprint"], current["fingerprint"]) + + def test_verifier_receipt_matches_separately_from_execution_checkpoint(self): + index = self.manager.add_task("Write then verify", checkpoint={ + "action": "write_file", "args": '{"path":"site/page.html","content":"ok"}', + }, acceptance=[{"action": "read_file", "args": '{"path":"site/page.html"}'}]) + task = self.manager.tasks[index] + self.assertGreater(self.manager._receipt_match_score( + task, "read_file", '{"path":"site/page.html"}', + ), 0) + + def test_task_completion_requires_every_strategic_acceptance_id(self): + index = self.manager.add_task("Verify both criteria", acceptance=["both checks pass"]) + task = self.manager.tasks[index] + task["checkpoint"]["strategic_plan"] = {"acceptance_ids": ["criterion-a", "criterion-b"]} + task["evidence"] = [{"success": True, "acceptance": "first", "acceptance_ids": ["criterion-a"]}] + self.manager.request_completion(index) + self.manager.reconcile_completions({task["id"]}) + self.assertEqual(task["status"], "pending") + def test_active_unrelated_working_plan_is_preserved_when_strategic_acceptance_is_rejected(self): self.manager.update_plan(complexity="complex", tasks=[{ "description": "Existing work", "acceptance": ["Existing work is done"], @@ -1182,7 +1281,7 @@ def test_bounded_write_objective_allows_mutation_with_a_separate_read_verifier(s controller = InteractionController(self.store, workspace_id="workspace", session_id="session") proposal = controller.propose_plan({ "title": "Write page", "summary": "Write the page.", "bounds": { - "workspace_paths": ["site"], "capabilities": ["write"], + "workspace_paths": ["site"], "capabilities": ["read", "write"], "effects": ["filesystem", "read_only"], "risk_ceiling": "normal", }, "steps": [{"id": "page", "title": "Write page", "description": "Write the page.", "acceptance": [{"criterion": "Page can be read", "verification": { @@ -1221,7 +1320,7 @@ def test_explicit_execution_contract_rejects_changed_write_arguments(self): approved = {"path": "site/index.html", "content": "approved"} proposal = controller.propose_plan({ "title": "Write page", "summary": "Write the approved content.", "bounds": { - "workspace_paths": ["site"], "capabilities": ["write"], + "workspace_paths": ["site"], "capabilities": ["read", "write"], "effects": ["filesystem", "read_only"], "risk_ceiling": "normal", }, "steps": [{"id": "page", "title": "Write page", "description": "Write the page.", "execution": {"action": "write_file", "arguments": approved}, From 4b28daea6cfba1ee070a41f9654ff84f1ac5aba0 Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 17 Oct 2026 13:40:49 +0800 Subject: [PATCH 15/16] docs: record strategic plan review follow-up --- docs/superpowers/plans/2026-10-16-v3-issue-245.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index 32dd621..69b0a93 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -161,3 +161,5 @@ The update remains limited to #245. Leave PR #285 and issue #245 open and unmerg - [x] Reject Strategic Plan acceptance while an unrelated active Working Plan exists, preserving that plan and its tasks. - [x] Derive new proposal versions from the latest historical proposal for the executing plan, including cancelled revisions, so stale approval identifiers cannot target a replacement. - [ ] Run final focused/full CI, refresh automated review, and resolve all remaining review threads before continuing to #246. + +Follow-up review fixes: pathless reads now authorize their effective `.` path; explicit file objectives match only that exact file; cancelled current objectives remain completion blockers; contract actions are checked against normalized bounds; verifier receipts may match separately from execution actions; and every required acceptance ID is enforced. Focused interaction and execution-receipt suites pass. GitHub lists `piggyevan123@gmail.com` as the verified address for the signing key; this follow-up uses that identity. From a0648c6a8446e09a94f28d440178bc110e177cdc Mon Sep 17 00:00:00 2001 From: Evan Date: Sat, 17 Oct 2026 13:42:52 +0800 Subject: [PATCH 16/16] docs: record baseline provider timeout result --- docs/superpowers/plans/2026-10-16-v3-issue-245.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/superpowers/plans/2026-10-16-v3-issue-245.md b/docs/superpowers/plans/2026-10-16-v3-issue-245.md index 69b0a93..a7ececb 100644 --- a/docs/superpowers/plans/2026-10-16-v3-issue-245.md +++ b/docs/superpowers/plans/2026-10-16-v3-issue-245.md @@ -163,3 +163,5 @@ The update remains limited to #245. Leave PR #285 and issue #245 open and unmerg - [ ] Run final focused/full CI, refresh automated review, and resolve all remaining review threads before continuing to #246. Follow-up review fixes: pathless reads now authorize their effective `.` path; explicit file objectives match only that exact file; cancelled current objectives remain completion blockers; contract actions are checked against normalized bounds; verifier receipts may match separately from execution actions; and every required acceptance ID is enforced. Focused interaction and execution-receipt suites pass. GitHub lists `piggyevan123@gmail.com` as the verified address for the signing key; this follow-up uses that identity. + +Local full-suite status: 879 of 880 tests passed; the remaining existing 50 ms provider transport-timeout test also fails unchanged on `main` (`146bb19`) because no request reaches the fixture before the deadline. The final-head CI workflow dispatch is running; review fixes themselves pass their focused suites.