From c39dcabc2297e6fa5dffd2b71e14fe277c2b8f51 Mon Sep 17 00:00:00 2001 From: Tom Softreck Date: Sun, 6 Sep 2026 20:56:53 +0200 Subject: [PATCH] fix(autonomy): verify shell work before completing tickets --- project/ticket-095/README.md | 12 ++++ project/ticket-095/ai-codex-logs.txt | 1 + project/ticket-095/ai-codex.md | 1 + project/ticket-095/changelog.md | 1 + project/ticket-095/intent.json | 76 +++++++++++++++++++++++ project/ticket-095/preprompt.md | 1 + src/koru/autonomy/shell_drive_finalize.py | 63 ++++++++++++------- tests/test_shell_drive_finalize.py | 46 ++++++++++++++ 8 files changed, 178 insertions(+), 23 deletions(-) create mode 100644 project/ticket-095/README.md create mode 100644 project/ticket-095/ai-codex-logs.txt create mode 100644 project/ticket-095/ai-codex.md create mode 100644 project/ticket-095/changelog.md create mode 100644 project/ticket-095/intent.json create mode 100644 project/ticket-095/preprompt.md diff --git a/project/ticket-095/README.md b/project/ticket-095/README.md new file mode 100644 index 00000000..cef64796 --- /dev/null +++ b/project/ticket-095/README.md @@ -0,0 +1,12 @@ +# Ticket 095: Verify shell task outcome before finalization + +- **Status**: IN_PROGRESS +- **Workflow state**: PUBLICATION +- **Owner**: codex + +## Acceptance criteria +- [x] AC-01: Preserve actual shell output; explicit no-edit or permission-denied results cannot auto-close verified tickets. +- [x] AC-02: Verification succeeds before done; missing or failed verification never transiently completes the ticket. +- [x] AC-03: Regression tests and managed stack gates pass before protected publication. + +Validation: 31 finalization/post-run tests pass; Ruff, governance, Docker engine/Compose and whitespace checks pass. Negative prose is a conservative veto, not positive acceptance evidence. Per-ticket material delivery binding remains separate work. diff --git a/project/ticket-095/ai-codex-logs.txt b/project/ticket-095/ai-codex-logs.txt new file mode 100644 index 00000000..f8772ca7 --- /dev/null +++ b/project/ticket-095/ai-codex-logs.txt @@ -0,0 +1 @@ +Regression validation in progress. diff --git a/project/ticket-095/ai-codex.md b/project/ticket-095/ai-codex.md new file mode 100644 index 00000000..e89b93df --- /dev/null +++ b/project/ticket-095/ai-codex.md @@ -0,0 +1 @@ +SESSION_EXECUTION_AUTHORIZATION: user requested continuing investigation and fixes; existing session authorizes protected publication. Scope is finalization only; do not widen runtime authority. diff --git a/project/ticket-095/changelog.md b/project/ticket-095/changelog.md new file mode 100644 index 00000000..3a044795 --- /dev/null +++ b/project/ticket-095/changelog.md @@ -0,0 +1 @@ +Preserve executor output and verify before completion. diff --git a/project/ticket-095/intent.json b/project/ticket-095/intent.json new file mode 100644 index 00000000..abc95d03 --- /dev/null +++ b/project/ticket-095/intent.json @@ -0,0 +1,76 @@ +{ + "schema": "new-project.intent/v3", + "ticket": "ticket-095", + "summary": "Verify shell task outcome before finalization", + "workstream": "application", + "classification": { + "kind": "BUG", + "priority": "P1", + "origin": "regression" + }, + "allowedPaths": [ + "src/koru/autonomy/shell_drive_finalize.py", + "tests/test_shell_drive_finalize.py", + "project/ticket-095/**" + ], + "forbiddenPaths": [ + "project/ticket-*/user-*.md" + ], + "stacks": [ + "python", + "docker" + ], + "dependsOn": [], + "conflictsWith": [], + "integrationTicket": null, + "delivery": { + "acceptedBaseSha": "ab9dba9ec5b42ab68430ac96fd5b825bdbf665f0", + "targetBranch": "main", + "outcome": "Preserve shell outcome and verify before ticket completion; reject explicit unsuccessful execution; protected publication.", + "nonGoals": [ + "No widening runtime permissions or changing project verification commands" + ], + "complexity": "S", + "estimatedMinutes": 45, + "budgets": { + "maxImplementationFiles": 2, + "maxAffectedComponents": 1, + "maxPublicInterfaceChanges": 1, + "maxRuntimeDependencies": 0 + }, + "architecture": { + "status": "accepted", + "decision": "Keep verification before done and preserve negative execution evidence.", + "components": [ + { + "name": "shell-finalization", + "paths": [ + "src/koru/autonomy/shell_drive_finalize.py", + "tests/test_shell_drive_finalize.py" + ] + } + ], + "responsibilityChanges": false, + "interfaceChanges": [], + "dataChanges": [], + "ui": { + "impact": "none", + "states": [], + "evidence": [] + }, + "rollback": "Revert protected merge." + }, + "runtimeDependencies": [], + "validation": [ + { + "criterion": "AC-03", + "commands": [ + "python -m pytest -q tests/test_shell_drive_finalize.py", + "./project/governance-check.sh", + "docker compose config --quiet" + ], + "evidence": "Finalization regression tests and managed gates" + } + ] + } +} diff --git a/project/ticket-095/preprompt.md b/project/ticket-095/preprompt.md new file mode 100644 index 00000000..9c1f4435 --- /dev/null +++ b/project/ticket-095/preprompt.md @@ -0,0 +1 @@ +Implement only the accepted shell finalization scope. diff --git a/src/koru/autonomy/shell_drive_finalize.py b/src/koru/autonomy/shell_drive_finalize.py index ad879190..8c3b9648 100644 --- a/src/koru/autonomy/shell_drive_finalize.py +++ b/src/koru/autonomy/shell_drive_finalize.py @@ -1,19 +1,15 @@ """Auto-finalize planfile tickets after a successful shell-client drive. -IDE-chat lanes are asynchronous: the drive submits a prompt and the loop -waits for the editor agent to respond, so the ticket must stay open. The -tillm shell lane (claude-code, codex, aider, …) is synchronous — when the -drive returns ``ok`` the vendor CLI has already finished editing the repo. -Leaving the ticket in ``waiting_input`` then requires a human to close it, -which defeats the autonomous loop (2026-07-05: two god-module refactors -completed by claude-code sat open until an operator intervened). +IDE-chat lanes submit asynchronously; shell clients return synchronously. +Transport success alone does not establish task completion. Preserve execution +output and refuse automatic closure when it explicitly reports unsuccessful work. Policy via ``KORU_SHELL_DRIVE_AUTODONE``: -- ``verified`` (default): append the agent reply as a ticket note, mark the - ticket done, then run ``queue.post_run_verify`` commands (``koru.yaml``). - A red verify reopens/blocks the ticket via the existing policy. When no - verify commands are configured the ticket is left open (note only) — an +- ``verified`` (default): append the agent reply as a ticket note, + run ``queue.post_run_verify`` commands (``koru.yaml``), then mark the + ticket done only after successful verification. A red verify uses the existing + policy. When no verify commands are configured the ticket is left open (note only) — an agent's exit code alone is not proof of done. - ``always``: note + done without verification (trust the agent). - ``off``: note only. @@ -22,6 +18,7 @@ from __future__ import annotations import os +import re from collections.abc import Callable from pathlib import Path from typing import Any @@ -172,7 +169,7 @@ def note_provider_exhaustion( def _reply_note(reply: dict[str, Any]) -> str: - message = reply.get("message") or reply.get("stdout") or reply.get("output") or "" + message = reply.get("stdout") or reply.get("output") or reply.get("message") or "" if isinstance(message, bytes): message = message.decode("utf-8", errors="replace") text = str(message).strip() @@ -188,6 +185,20 @@ def _reply_note(reply: dict[str, Any]) -> str: return f"{note}\n{switch}" if switch else note + +def _reports_unsuccessful_work(reply: dict[str, Any]) -> bool: + """Negative execution evidence vetoes auto-done; prose never proves success.""" + text = "\n".join( + value.decode("utf-8", errors="replace") if isinstance(value, bytes) else str(value) + for key in ("stdout", "output", "message", "stderr") + if (value := reply.get(key)) + ) + return bool(re.search( + r"\b(?:no files (?:were )?modified|no changes (?:were )?made|" + r"(?:edit|write) permission denied|permission denied (?:for|to) (?:edit|write))\b", + text, flags=re.IGNORECASE, + )) + def _eligible_for_finalize( *, ok: bool, @@ -244,7 +255,7 @@ def _mark_done( return True -def _run_post_done_verify( +def _run_pre_done_verify( project: Path, ticket_id: str, *, @@ -263,6 +274,9 @@ def _run_post_done_verify( config=verify_config, planfile_runner=runner, ) + if len(outcomes) != 1 or outcomes[0].get("ticket_id") != ticket_id: + _hp(f" shell-drive finalize: {ticket_id} verification receipt missing or mismatched") + return "verify_failed:missing_receipt" failed = [o for o in outcomes if not o.get("ok")] if failed: action = str(failed[0].get("action") or "reopened") @@ -271,8 +285,7 @@ def _run_post_done_verify( "(agent work did not pass post_run_verify)", ) return f"verify_failed:{action}" - _hp(f" shell-drive finalize: {ticket_id} done + verified green") - return "done_verified" + return "verified" def finalize_shell_drive_ticket( @@ -316,11 +329,19 @@ def finalize_shell_drive_ticket( _hp(f" shell-drive finalize: note appended to {ticket_id} (autodone=off)") return "noted" - verify_config = None + if _reports_unsuccessful_work(reply): + _hp(f" shell-drive finalize: {ticket_id} left open — execution reports unsuccessful work") + return "noted_unsuccessful" + if policy == "verified": verify_config, early = _load_verify_config_or_skip(project, ticket_id, _hp=_hp) if early is not None: return early + verification = _run_pre_done_verify( + project, ticket_id, verify_config=verify_config, runner=run_process, _hp=_hp, + ) + if verification != "verified": + return verification if not _mark_done( project, @@ -335,13 +356,9 @@ def finalize_shell_drive_ticket( _hp(f" shell-drive finalize: {ticket_id} marked done (autodone=always)") return "done" - return _run_post_done_verify( - project, - ticket_id, - verify_config=verify_config, - runner=run_process, - _hp=_hp, - ) + _hp(f" shell-drive finalize: {ticket_id} done + verified green") + return "done_verified" + __all__ = [ diff --git a/tests/test_shell_drive_finalize.py b/tests/test_shell_drive_finalize.py index 9abfc82c..8712721f 100644 --- a/tests/test_shell_drive_finalize.py +++ b/tests/test_shell_drive_finalize.py @@ -299,3 +299,49 @@ def test_exhaustion_reported_live(self, tmp_path, planfile): } assert self._note(tmp_path, reply, hp=lines.append) == "noted_exhaustion" assert any("provider-exhausted" in line for line in lines) + + +@pytest.mark.parametrize('output', [ + 'Edit permission denied. NO FILES MODIFIED. NO TESTS RUN.', + 'No changes were made.', + b'NO FILES WERE MODIFIED.', +]) +def test_negative_work_evidence_vetoes_transport_success(tmp_path, planfile, monkeypatch, output): + monkeypatch.setenv('KORU_SHELL_DRIVE_AUTODONE', 'always') + assert _finalize(tmp_path, reply={ + 'ok': True, 'message': 'completed', 'stdout': output, 'client_id': 'claude-code', + }) == 'noted_unsuccessful' + assert planfile.commands() == ['ticket show', 'ticket update'] + assert 'completed' not in planfile.calls[-1][-1] + + +def test_verified_checks_run_before_done(tmp_path, planfile, monkeypatch): + config = verify_mod.PostRunVerifyConfig(enabled=True, commands=('true',)) + monkeypatch.setattr(verify_mod, 'load_post_run_verify_config', lambda p: config) + + def verify(project, ids, **kwargs): + assert 'ticket done' not in planfile.commands() + return [{'ticket_id': i, 'ok': True, 'action': 'verified'} for i in ids] + + monkeypatch.setattr(verify_mod, 'verify_completed_tickets', verify) + assert _finalize(tmp_path) == 'done_verified' + assert planfile.commands()[-1] == 'ticket done' + + +@pytest.mark.parametrize('receipts', [[], [{'ticket_id': 'OTHER', 'ok': True}]]) +def test_missing_ticket_verification_never_closes(tmp_path, planfile, monkeypatch, receipts): + config = verify_mod.PostRunVerifyConfig(enabled=True, commands=('true',)) + monkeypatch.setattr(verify_mod, 'load_post_run_verify_config', lambda p: config) + monkeypatch.setattr(verify_mod, 'verify_completed_tickets', lambda *a, **k: receipts) + assert _finalize(tmp_path) == 'verify_failed:missing_receipt' + assert 'ticket done' not in planfile.commands() + + +def test_failed_verification_never_transiently_completes(tmp_path, planfile, monkeypatch): + config = verify_mod.PostRunVerifyConfig(enabled=True, commands=('false',)) + monkeypatch.setattr(verify_mod, 'load_post_run_verify_config', lambda p: config) + monkeypatch.setattr(verify_mod, 'verify_completed_tickets', lambda *a, **k: [ + {'ticket_id': 'STARTER-1', 'ok': False, 'action': 'reopened'}, + ]) + assert _finalize(tmp_path) == 'verify_failed:reopened' + assert 'ticket done' not in planfile.commands()