diff --git a/nanobot/agent/tools/exec_session.py b/nanobot/agent/tools/exec_session.py index b0d79978b..2ff53fddc 100644 --- a/nanobot/agent/tools/exec_session.py +++ b/nanobot/agent/tools/exec_session.py @@ -17,6 +17,11 @@ from nanobot.agent.tools.schema import ( StringSchema, tool_parameters_schema, ) +from nanobot.agent.verification_state import ( + analyze_verification_result, + append_verification_feedback, + record_verification_observation, +) DEFAULT_YIELD_MS = 1000 MAX_YIELD_MS = 30_000 @@ -351,6 +356,20 @@ def format_session_poll(session_id: str, poll: _SessionPoll) -> str: return "\n".join(parts) if parts else "(no output yet)" +def _format_poll_with_verification(session_id: str, poll: _SessionPoll) -> str: + result = format_session_poll(session_id, poll) + if not poll.done: + return result + analysis = analyze_verification_result( + command="", + output=result, + exit_code=poll.exit_code, + timed_out=poll.timed_out, + ) + record_verification_observation(current_request_session_key(), analysis) + return append_verification_feedback(result, analysis) + + @tool_parameters( tool_parameters_schema( session_id=StringSchema("Session id returned by exec when yield_time_ms is used."), @@ -492,7 +511,7 @@ class WriteStdinTool(Tool): max_output_chars=output_limit, owner_session_key=current_request_session_key(), ) - return format_session_poll(session_id, poll) + return _format_poll_with_verification(session_id, poll) except KeyError: return f"Error: exec session not found: {session_id}" except Exception as exc: @@ -532,10 +551,10 @@ class WriteStdinTool(Tool): joined = "".join(aggregate) if wait_for in joined: poll.output = joined - return format_session_poll(session_id, poll) + return _format_poll_with_verification(session_id, poll) if poll.done or remaining_ms <= 0: poll.output = "".join(aggregate) - result = format_session_poll(session_id, poll) + result = _format_poll_with_verification(session_id, poll) if wait_for not in poll.output: result += f"\nWait target not observed: {wait_for!r}" return result diff --git a/nanobot/agent/tools/long_task.py b/nanobot/agent/tools/long_task.py index 12fcec174..367f282e0 100644 --- a/nanobot/agent/tools/long_task.py +++ b/nanobot/agent/tools/long_task.py @@ -23,6 +23,11 @@ from typing import TYPE_CHECKING, Any from nanobot.agent.tools.base import Tool, tool_parameters from nanobot.agent.tools.context import ContextAware, RequestContext from nanobot.agent.tools.schema import StringSchema, tool_parameters_schema +from nanobot.agent.verification_state import ( + clear_verification_observation, + format_completion_gate_message, + latest_verification_observation, +) from nanobot.bus.runtime_events import GoalStateChanged, RuntimeEventBus, RuntimeEventContext from nanobot.session.goal_state import ( GOAL_STATE_KEY, @@ -187,6 +192,29 @@ class LongTaskTool(Tool, _GoalToolsMixin): max_length=8000, nullable=True, ), + verification_summary=StringSchema( + "For coding or file-producing tasks, summarize how the work was verified. " + "Mention the most relevant test/check command and whether it passed. " + "If no verification was possible, say why.", + max_length=4000, + nullable=True, + ), + commands_run=StringSchema( + "Optional concise list of verification/build commands run before completion.", + max_length=4000, + nullable=True, + ), + artifacts_created=StringSchema( + "Optional concise list of files, outputs, or artifacts created.", + max_length=4000, + nullable=True, + ), + remaining_failures=StringSchema( + "Known unresolved failures, if intentionally stopping before success. " + "Leave empty when verification passes.", + max_length=4000, + nullable=True, + ), required=[], ) ) @@ -222,30 +250,67 @@ class CompleteGoalTool(Tool, _GoalToolsMixin): return ( "End bookkeeping for the active sustained goal. " "Use when the objective is fully achieved and verified—recap what was delivered. " + "For coding/file-producing tasks, run the smallest reliable verification first and include " + "verification_summary / commands_run / artifacts_created. " "Also call when the user cancels, redirects, or replaces the goal: recap must reflect " "what actually happened (not necessarily success). " + "If recent verification failed and no later verification passed, this tool will ask you to " + "continue fixing unless remaining_failures describes an intentional incomplete stop. " "If no goal is active, the tool reports that and leaves metadata unchanged." ) - async def execute(self, recap: str | None = None, **kwargs: Any) -> str: + async def execute( + self, + recap: str | None = None, + verification_summary: str | None = None, + commands_run: str | None = None, + artifacts_created: str | None = None, + remaining_failures: str | None = None, + **kwargs: Any, + ) -> str: sess = self._session() if sess is None: return "Error: complete_goal requires an active chat session." + + session_key = self._request_ctx.get().session_key if self._request_ctx.get() else None + observation = latest_verification_observation(session_key) + if ( + observation is not None + and observation.analysis.status == "failed" + and not _has_meaningful_remaining_failures(remaining_failures) + ): + return format_completion_gate_message(observation) + prior = parse_goal_state(goal_state_raw(sess.metadata)) if not isinstance(prior, dict) or prior.get("status") != "active": return "No active goal to complete." ended = _iso_now() - sess.metadata[GOAL_STATE_KEY] = { + completed = { **prior, "status": "completed", "completed_at": ended, "recap": (recap or "").strip(), } + if verification_summary: + completed["verification_summary"] = verification_summary.strip() + if commands_run: + completed["commands_run"] = commands_run.strip() + if artifacts_created: + completed["artifacts_created"] = artifacts_created.strip() + if remaining_failures: + completed["remaining_failures"] = remaining_failures.strip() + sess.metadata[GOAL_STATE_KEY] = completed discard_legacy_goal_state_key(sess.metadata) self._sessions.save(sess) + clear_verification_observation(session_key) await self._publish_goal_state_changed(sess.metadata) tail = (recap or "").strip() if tail: return f"Goal marked complete ({ended}). Recap:\n{tail}" return f"Goal marked complete ({ended})." + + +def _has_meaningful_remaining_failures(value: str | None) -> bool: + text = (value or "").strip().lower() + return bool(text and text not in {"none", "no", "n/a", "na", "no remaining failures"}) diff --git a/nanobot/agent/tools/shell.py b/nanobot/agent/tools/shell.py index 28e2d0bf9..f23a82954 100644 --- a/nanobot/agent/tools/shell.py +++ b/nanobot/agent/tools/shell.py @@ -33,6 +33,11 @@ from nanobot.agent.tools.schema import ( StringSchema, tool_parameters_schema, ) +from nanobot.agent.verification_state import ( + analyze_verification_result, + append_verification_feedback, + record_verification_observation, +) from nanobot.config.paths import get_media_dir from nanobot.config_base import Base from nanobot.security.workspace_access import current_scope_allows_loopback, current_tool_workspace @@ -283,7 +288,15 @@ class ExecTool(Tool): ) except asyncio.TimeoutError: await self._kill_process(process) - return f"Error: Command timed out after {prepared.timeout} seconds" + result = f"Error: Command timed out after {prepared.timeout} seconds" + analysis = analyze_verification_result( + command=prepared.command, + output=result, + exit_code=None, + timed_out=True, + ) + record_verification_observation(current_request_session_key(), analysis) + return append_verification_feedback(result, analysis) except asyncio.CancelledError: await self._kill_process(process) raise @@ -311,7 +324,13 @@ class ExecTool(Tool): + result[-half:] ) - return result + analysis = analyze_verification_result( + command=prepared.command, + output=result, + exit_code=process.returncode, + ) + record_verification_observation(current_request_session_key(), analysis) + return append_verification_feedback(result, analysis) except Exception as e: return f"Error executing command: {str(e)}" @@ -339,7 +358,17 @@ class ExecTool(Tool): MAX_OUTPUT_CHARS, ), ) - return format_session_poll(session_id, poll) + result = format_session_poll(session_id, poll) + if poll.done: + analysis = analyze_verification_result( + command=prepared.command, + output=result, + exit_code=poll.exit_code, + timed_out=poll.timed_out, + ) + record_verification_observation(current_request_session_key(), analysis) + return append_verification_feedback(result, analysis) + return result except Exception as exc: return f"Error executing command: {exc}" diff --git a/nanobot/agent/verification_state.py b/nanobot/agent/verification_state.py new file mode 100644 index 000000000..9db53c424 --- /dev/null +++ b/nanobot/agent/verification_state.py @@ -0,0 +1,241 @@ +"""Lightweight verification-result detection for coding workflows.""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from typing import Literal + +VerificationStatus = Literal["passed", "failed"] + + +@dataclass(frozen=True, slots=True) +class VerificationAnalysis: + """Structured summary of a command that appears to be verification.""" + + status: VerificationStatus + command: str + exit_code: int | None + failed_tests: tuple[str, ...] = () + primary_errors: tuple[str, ...] = () + missing_artifacts: tuple[str, ...] = () + timed_out: bool = False + + +@dataclass(frozen=True, slots=True) +class VerificationObservation: + """Latest verification signal observed for a session.""" + + analysis: VerificationAnalysis + sequence: int + + +_OBSERVATIONS: dict[str, VerificationObservation] = {} +_SEQUENCE = 0 + +_TEST_COMMAND_RE = re.compile( + r"(?ix)" + r"(" + r"\bpytest\b|\bpy\.test\b|\bunittest\b|\bnosetests\b|" + r"\btest_outputs\.py\b|\brun_tests?(?:\.sh|\.py)?\b|" + r"\bnpm\s+(?:run\s+)?test\b|\byarn\s+test\b|\bpnpm\s+test\b|" + r"\bcargo\s+test\b|\bgo\s+test\b|\bctest\b|" + r"\bmake\s+(?:[^;&|]*\s+)?test\b" + r")" +) +_FAILURE_RE = re.compile( + r"(?im)" + r"(" + r"^FAILED\s+|" + r"\b\d+\s+failed\b|" + r"\bAssertionError\b|" + r"\bFileNotFoundError\b|" + r"\bTimeoutError\b|" + r"\bError:\s+Command timed out\b|" + r"\bFAILURES?\b|" + r"\bTEST FAILED\b" + r")" +) +_SUCCESS_RE = re.compile( + r"(?im)" + r"(" + r"\b\d+\s+passed\b|" + r"\bOK\b|" + r"\bTEST PASSED\b|" + r"\bExit code:\s*0\b" + r")" +) +_FAILED_TEST_RE = re.compile(r"(?m)^FAILED\s+([^\s]+)") +_PYTEST_SHORT_RE = re.compile(r"(?m)^_{3,}\s+([A-Za-z0-9_./:-]+)\s+_{3,}$") +_ERROR_LINE_RE = re.compile( + r"(?m)" + r"^\s*(?:E\s+)?(" + r"(?:AssertionError|FileNotFoundError|TimeoutError|ValueError|TypeError|RuntimeError)" + r"(?::[^\n]*)?|" + r"assert\s+[^\n]+|" + r"Error:\s+[^\n]+|" + r"TEST FAILED[^\n]*" + r")" +) +_MISSING_PATH_RE = re.compile( + r"(?i)" + r"(?:No such file or directory:\s*['\"]([^'\"]+)['\"]|" + r"(?:file|path)\s+([^\s'\"]+)\s+does not exist|" + r"cannot open file\s+['\"]([^'\"]+)['\"])" +) + + +def analyze_verification_result( + *, + command: str, + output: str, + exit_code: int | None, + timed_out: bool = False, +) -> VerificationAnalysis | None: + """Return a verification summary when a command/output looks like a test.""" + + command = " ".join((command or "").split()) + looks_like_test_command = bool(_TEST_COMMAND_RE.search(command)) + failure_seen = bool(_FAILURE_RE.search(output)) + success_seen = bool(_SUCCESS_RE.search(output)) + + if not looks_like_test_command and not failure_seen: + return None + + if (timed_out and looks_like_test_command) or (exit_code not in (None, 0) and (looks_like_test_command or failure_seen)) or failure_seen: + return VerificationAnalysis( + status="failed", + command=command, + exit_code=exit_code, + failed_tests=_unique(_FAILED_TEST_RE.findall(output), limit=8), + primary_errors=_extract_primary_errors(output), + missing_artifacts=_extract_missing_artifacts(output), + timed_out=timed_out, + ) + + if looks_like_test_command and exit_code == 0 and success_seen: + return VerificationAnalysis( + status="passed", + command=command, + exit_code=exit_code, + ) + + return None + + +def append_verification_feedback(output: str, analysis: VerificationAnalysis | None) -> str: + """Append model-facing feedback for failed verification results.""" + + if analysis is None or analysis.status != "failed": + return output + + lines = [ + "", + "[Verification Feedback]", + "Verification status: failed.", + "Do not call complete_goal or present the task as finished until this is fixed and a verification passes.", + ] + if analysis.command: + lines.append(f"Command: {analysis.command[:240]}") + if analysis.exit_code is not None: + lines.append(f"Exit code: {analysis.exit_code}") + if analysis.timed_out: + lines.append("Failure type: command timeout") + if analysis.failed_tests: + lines.append("Failed tests:") + lines.extend(f"- {item}" for item in analysis.failed_tests) + if analysis.primary_errors: + lines.append("Primary errors:") + lines.extend(f"- {item}" for item in analysis.primary_errors) + if analysis.missing_artifacts: + lines.append("Missing artifacts:") + lines.extend(f"- {item}" for item in analysis.missing_artifacts) + lines.append("Next action: inspect the failing assertion, fix the implementation or artifact, then rerun the most specific verification command.") + lines.append("[/Verification Feedback]") + return output.rstrip() + "\n" + "\n".join(lines) + + +def record_verification_observation(session_key: str | None, analysis: VerificationAnalysis | None) -> None: + """Remember the latest verification signal for a session.""" + + if not session_key or analysis is None: + return + global _SEQUENCE + _SEQUENCE += 1 + _OBSERVATIONS[session_key] = VerificationObservation( + analysis=analysis, + sequence=_SEQUENCE, + ) + + +def latest_verification_observation(session_key: str | None) -> VerificationObservation | None: + if not session_key: + return None + return _OBSERVATIONS.get(session_key) + + +def clear_verification_observation(session_key: str | None) -> None: + if session_key: + _OBSERVATIONS.pop(session_key, None) + + +def format_completion_gate_message(observation: VerificationObservation) -> str: + """Build the complete_goal soft-gate message for unresolved failures.""" + + analysis = observation.analysis + lines = [ + "Recent verification appears to have failed, so the goal is not marked complete yet.", + "Continue fixing the task and rerun verification before completing.", + ] + if analysis.command: + lines.append(f"Last failed verification command: {analysis.command[:240]}") + if analysis.failed_tests: + lines.append("Failed tests: " + ", ".join(analysis.failed_tests[:5])) + if analysis.primary_errors: + lines.append("Primary error: " + analysis.primary_errors[0]) + if analysis.missing_artifacts: + lines.append("Missing artifact: " + analysis.missing_artifacts[0]) + lines.append( + "If you are intentionally stopping with known failures, call complete_goal again with remaining_failures describing them honestly." + ) + return "\n".join(lines) + + +def _extract_primary_errors(output: str) -> tuple[str, ...]: + candidates: list[str] = [] + for match in _ERROR_LINE_RE.findall(output): + text = " ".join(match.split()) + if text and text not in candidates: + candidates.append(text[:240]) + if len(candidates) >= 8: + break + if not candidates: + for match in _PYTEST_SHORT_RE.findall(output): + text = " ".join(match.split()) + if text and text not in candidates: + candidates.append(text[:240]) + if len(candidates) >= 4: + break + return tuple(candidates) + + +def _extract_missing_artifacts(output: str) -> tuple[str, ...]: + paths: list[str] = [] + for groups in _MISSING_PATH_RE.findall(output): + path = next((item for item in groups if item), "") + if path and path not in paths: + paths.append(path[:240]) + if len(paths) >= 8: + break + return tuple(paths) + + +def _unique(items: list[str], *, limit: int) -> tuple[str, ...]: + out: list[str] = [] + for item in items: + text = " ".join(item.split()) + if text and text not in out: + out.append(text[:240]) + if len(out) >= limit: + break + return tuple(out) diff --git a/nanobot/command/builtin.py b/nanobot/command/builtin.py index 4a34f894f..db13fc946 100644 --- a/nanobot/command/builtin.py +++ b/nanobot/command/builtin.py @@ -626,7 +626,7 @@ async def cmd_history(ctx: CommandContext) -> OutboundMessage: _GOAL_PROMPT_TEMPLATE = """The user declared a sustained objective for this thread. -Inspect or clarify if needed, then call `long_task` with the refined objective (and optional short ui_summary). Work proceeds as normal assistant turns using your usual tools. When the objective is fully done and verified, call `complete_goal` with a brief recap. If the user later cancels or changes direction, still call `complete_goal` with an honest recap (then `long_task` again only after there is no active goal). Do not use `long_task` / `complete_goal` for trivial one-shot answers. +Inspect or clarify if needed, then call `long_task` with the refined objective (and optional short ui_summary). Work proceeds as normal assistant turns using your usual tools. When the objective is fully done and verified, call `complete_goal` with a brief recap plus verification_summary / commands_run / artifacts_created when applicable. If the user later cancels or changes direction, still call `complete_goal` with an honest recap (then `long_task` again only after there is no active goal). Do not use `long_task` / `complete_goal` for trivial one-shot answers. Goal: {goal} diff --git a/nanobot/providers/openai_codex_provider.py b/nanobot/providers/openai_codex_provider.py index 64430cdc4..03f12fbb1 100644 --- a/nanobot/providers/openai_codex_provider.py +++ b/nanobot/providers/openai_codex_provider.py @@ -2,6 +2,7 @@ from __future__ import annotations +import ast import asyncio import hashlib import json @@ -26,6 +27,25 @@ from nanobot.providers.openai_responses import ( DEFAULT_CODEX_URL = "https://chatgpt.com/backend-api/codex/responses" DEFAULT_ORIGINATOR = "nanobot" +_RESPONSE_FAILED_PREFIX = "Response failed:" +_RETRYABLE_RESPONSE_FAILED_TOKENS = frozenset({ + "overloaded", + "overloaded_error", + "rate_limit_exceeded", + "request_limit_exceeded", + "requests_limit_exceeded", + "server_error", + "server_is_overloaded", + "service_unavailable", + "temporarily_unavailable", + "too_many_requests", +}) +_NON_RETRYABLE_RESPONSE_FAILED_TOKENS = frozenset({ + "content_filter", + "content_policy_violation", + "cyber_policy", + "safety_violation", +}) class OpenAICodexProvider(LLMProvider): @@ -246,6 +266,8 @@ def _codex_error_response(exc: Exception) -> LLMResponse: status_code = getattr(exc, "status_code", None) error_kind: str | None = None + error_type = getattr(exc, "error_type", None) + error_code = getattr(exc, "error_code", None) default_detail: str | None = None should_retry: bool | None = getattr(exc, "should_retry", None) @@ -265,12 +287,20 @@ def _codex_error_response(exc: Exception) -> LLMResponse: error_kind = "http" default_detail = "HTTP request failed" + failed_type, failed_code = _extract_response_failed_error(detail) + if failed_type or failed_code: + error_kind = error_kind or "provider" + error_type = failed_type or error_type + error_code = failed_code or error_code + if should_retry is None: + should_retry = _should_retry_response_failed(error_type, error_code, detail) + if status_code is not None and should_retry is None: retry_content = None if int(status_code) == 429 and isinstance(exc, _CodexHTTPError) else detail should_retry = _should_retry_status( int(status_code), - getattr(exc, "error_type", None), - getattr(exc, "error_code", None), + error_type, + error_code, retry_content, ) @@ -283,13 +313,56 @@ def _codex_error_response(exc: Exception) -> LLMResponse: retry_after=retry_after, error_status_code=int(status_code) if status_code is not None else None, error_kind=error_kind, - error_type=getattr(exc, "error_type", None), - error_code=getattr(exc, "error_code", None), + error_type=error_type, + error_code=error_code, error_retry_after_s=retry_after, error_should_retry=should_retry, ) +def _extract_response_failed_error(detail: str) -> tuple[str | None, str | None]: + """Extract provider semantic error fields from Responses SSE failures.""" + if _RESPONSE_FAILED_PREFIX not in detail: + return None, None + + payload = detail.split(_RESPONSE_FAILED_PREFIX, 1)[1].strip() + if not payload: + return None, None + + parsed: Any = None + try: + parsed = json.loads(payload) + except Exception: + try: + parsed = ast.literal_eval(payload) + except Exception: + parsed = None + + error_type, error_code = LLMProvider._extract_error_type_code(parsed or payload) + return error_type, error_code + + +def _should_retry_response_failed( + error_type: str | None, + error_code: str | None, + detail: str, +) -> bool | None: + semantic_tokens = { + token for token in ( + LLMProvider._normalize_error_token(error_type), + LLMProvider._normalize_error_token(error_code), + ) + if token is not None + } + if any(token in _NON_RETRYABLE_RESPONSE_FAILED_TOKENS for token in semantic_tokens): + return False + if any(token in _RETRYABLE_RESPONSE_FAILED_TOKENS for token in semantic_tokens): + return True + if LLMProvider._is_transient_error(detail): + return True + return None + + def _codex_log_summary(exc_type: str, response: LLMResponse) -> str: """Return a bounded diagnostic summary without request body or raw upstream payload.""" if response.error_status_code is not None: diff --git a/nanobot/skills/long-goal/SKILL.md b/nanobot/skills/long-goal/SKILL.md index d43c3de71..0d0065de4 100644 --- a/nanobot/skills/long-goal/SKILL.md +++ b/nanobot/skills/long-goal/SKILL.md @@ -26,7 +26,7 @@ Those belong to the execution phase after the marker is set. - **`long_task`** — Register **one** sustained objective per thread. Call it promptly once the user has asked for a sustained task. The `goal` should follow the idempotent-goal rules below, but it should be produced quickly from the user's request—not after a long hidden planning pass. -- **`complete_goal`** — Close bookkeeping for the **current** active goal. Call when work is **done**, **and also** when the user **cancels**, **changes direction**, or **replaces** the objective: use **`recap`** to state honestly what happened (e.g. cancelled, partially done, superseded). Then you may call **`long_task`** again for a **new** objective after the session shows no active goal (or after the user agrees to replace). +- **`complete_goal`** — Close bookkeeping for the **current** active goal. Call when work is **done**, **and also** when the user **cancels**, **changes direction**, or **replaces** the objective: use **`recap`** to state honestly what happened (e.g. cancelled, partially done, superseded). For coding or file-producing tasks, include **`verification_summary`**, **`commands_run`**, and **`artifacts_created`** when possible; if stopping with known unresolved issues, fill **`remaining_failures`** honestly. Then you may call **`long_task`** again for a **new** objective after the session shows no active goal (or after the user agrees to replace). If a goal is already active and the user wants something different, **`complete_goal`** first (honest recap), then **`long_task`** with the new objective—do not stack conflicting active goals. @@ -68,7 +68,7 @@ Use this when the goal is to **build or reshape a codebase** (app, service, tool 1. **Modular layout** — Split into **meaningful modules** (directories + files with clear responsibilities: entrypoints, domain logic, config, infra, CLI/UI routes, etc.). **Do not** default to dumping an entire project into one giant source file unless the user explicitly wants a minimal single-file artifact. 2. **Conventional structure** — Follow normal practice for that stack (separation of concerns, sensible naming, config vs code, reusable helpers). Aim for reviewable increments, not unreadable blobs. -3. **Verify as you go** — Run/format/lint/tests the project affords after meaningful chunks so the tree stays truthful; bake **checks or manual steps into the goal** when they matter. +3. **Verify as you go** — Run/format/lint/tests the project affords after meaningful chunks so the tree stays truthful; bake **checks or manual steps into the goal** when they matter. Before `complete_goal`, run the smallest reliable verification you can and summarize it in `verification_summary`. ## Look things up instead of guessing diff --git a/tests/agent/test_verification_state.py b/tests/agent/test_verification_state.py new file mode 100644 index 000000000..f06e41447 --- /dev/null +++ b/tests/agent/test_verification_state.py @@ -0,0 +1,54 @@ +from __future__ import annotations + +from nanobot.agent.verification_state import ( + analyze_verification_result, + append_verification_feedback, +) + + +def test_analyze_pytest_failure_extracts_actionable_summary(): + output = """\ +FAILED ../tests/test_outputs.py::test_regex_matches_dates - AssertionError: Expected dates +E AssertionError: Expected ['2025-01-09'], but got ['bad'] +E FileNotFoundError: [Errno 2] No such file or directory: '/app/out.txt' +============================== 1 failed in 0.05s =============================== +Exit code: 1 +""" + + analysis = analyze_verification_result( + command="pytest /tests/test_outputs.py", + output=output, + exit_code=1, + ) + + assert analysis is not None + assert analysis.status == "failed" + assert analysis.failed_tests == ("../tests/test_outputs.py::test_regex_matches_dates",) + assert any("AssertionError" in item for item in analysis.primary_errors) + assert "/app/out.txt" in analysis.missing_artifacts + + +def test_append_verification_feedback_tells_agent_not_to_finish(): + analysis = analyze_verification_result( + command="python /app/test_outputs.py", + output="FAILED test_outputs.py::test_file\nAssertionError: missing\nExit code: 1", + exit_code=1, + ) + + feedback = append_verification_feedback("raw output\nExit code: 1", analysis) + + assert "[Verification Feedback]" in feedback + assert "Do not call complete_goal" in feedback + assert "Next action" in feedback + + +def test_analyze_passing_test_records_success_without_feedback(): + analysis = analyze_verification_result( + command="pytest", + output="============================== 3 passed in 0.10s ==============================\nExit code: 0", + exit_code=0, + ) + + assert analysis is not None + assert analysis.status == "passed" + assert append_verification_feedback("ok", analysis) == "ok" diff --git a/tests/agent/tools/test_long_task.py b/tests/agent/tools/test_long_task.py index 03bd91d8b..ecfa34be4 100644 --- a/tests/agent/tools/test_long_task.py +++ b/tests/agent/tools/test_long_task.py @@ -13,6 +13,11 @@ from nanobot.agent.tools.long_task import ( CompleteGoalTool, LongTaskTool, ) +from nanobot.agent.verification_state import ( + VerificationAnalysis, + clear_verification_observation, + record_verification_observation, +) from nanobot.bus.queue import MessageBus from nanobot.bus.runtime_events import RuntimeEventBus from nanobot.session.goal_state import GOAL_STATE_KEY @@ -192,6 +197,66 @@ async def test_complete_goal_without_active_is_noop_message(tmp_path): assert "No active" in out +@pytest.mark.asyncio +async def test_complete_goal_blocks_unresolved_verification_failure(tmp_path): + sm = SessionManager(tmp_path) + lt, cg = _tools(sm) + await lt.execute(goal="Fix the tests") + record_verification_observation( + "websocket:c1", + VerificationAnalysis( + status="failed", + command="pytest /tests/test_outputs.py", + exit_code=1, + failed_tests=("test_outputs.py::test_output",), + primary_errors=("AssertionError: wrong output",), + ), + ) + + out = await cg.execute(recap="Done.") + + assert "not marked complete" in out + assert "test_outputs.py::test_output" in out + assert sm.get_or_create("websocket:c1").metadata[GOAL_STATE_KEY]["status"] == "active" + clear_verification_observation("websocket:c1") + + +@pytest.mark.asyncio +async def test_complete_goal_allows_after_later_successful_verification(tmp_path): + sm = SessionManager(tmp_path) + lt, cg = _tools(sm) + await lt.execute(goal="Fix the tests") + record_verification_observation( + "websocket:c1", + VerificationAnalysis( + status="failed", + command="pytest /tests/test_outputs.py", + exit_code=1, + failed_tests=("test_outputs.py::test_output",), + ), + ) + record_verification_observation( + "websocket:c1", + VerificationAnalysis( + status="passed", + command="pytest /tests/test_outputs.py", + exit_code=0, + ), + ) + + out = await cg.execute( + recap="Done.", + verification_summary="pytest /tests/test_outputs.py passed", + commands_run="pytest /tests/test_outputs.py", + artifacts_created="/app/out.txt", + ) + + assert "marked complete" in out + blob = sm.get_or_create("websocket:c1").metadata[GOAL_STATE_KEY] + assert blob["status"] == "completed" + assert blob["verification_summary"] == "pytest /tests/test_outputs.py passed" + + @pytest.mark.asyncio async def test_long_task_skips_ws_publish_without_bus(tmp_path): sm = SessionManager(tmp_path) diff --git a/tests/providers/test_openai_codex_provider.py b/tests/providers/test_openai_codex_provider.py index cbd3ed488..ba677202c 100644 --- a/tests/providers/test_openai_codex_provider.py +++ b/tests/providers/test_openai_codex_provider.py @@ -303,6 +303,37 @@ async def test_codex_http_error_preserves_status_and_retry_after(monkeypatch) -> assert response.error_should_retry is True +def test_codex_response_failed_server_error_is_retryable() -> None: + response = _codex_error_response( + RuntimeError( + "Response failed: {'type': 'server_error', 'code': 'server_error', " + "'message': 'The server had an error while processing your request.'}" + ) + ) + + assert response.finish_reason == "error" + assert response.error_kind == "provider" + assert response.error_type == "server_error" + assert response.error_code == "server_error" + assert response.error_should_retry is True + assert provider_base.LLMProvider._is_transient_response(response) is True + + +def test_codex_response_failed_cyber_policy_is_not_retryable() -> None: + response = _codex_error_response( + RuntimeError( + "Response failed: {'type': 'invalid_request_error', 'code': 'cyber_policy', " + "'message': 'Request denied.'}" + ) + ) + + assert response.error_kind == "provider" + assert response.error_type == "invalid_request_error" + assert response.error_code == "cyber_policy" + assert response.error_should_retry is False + assert provider_base.LLMProvider._is_transient_response(response) is False + + @pytest.mark.asyncio async def test_codex_http_diagnostic_log_omits_raw_body(monkeypatch) -> None: log_capture = _capture_codex_warnings(monkeypatch) diff --git a/tests/tools/test_exec_session_tools.py b/tests/tools/test_exec_session_tools.py index 3ef3f37b8..656049a96 100644 --- a/tests/tools/test_exec_session_tools.py +++ b/tests/tools/test_exec_session_tools.py @@ -104,6 +104,23 @@ def test_exec_one_shot_accepts_max_output_tokens_alias(tmp_path): assert "Exit code: 0" in result +def test_exec_adds_verification_feedback_for_test_failures(tmp_path): + async def run() -> str: + tool = ExecTool(working_dir=str(tmp_path), timeout=5) + command = _python_command( + "print('FAILED test_outputs.py::test_answer - AssertionError: wrong'); " + "print('AssertionError: wrong'); raise SystemExit(1)" + ) + return await tool.execute(command=command) + + result = asyncio.run(run()) + + assert "Exit code: 1" in result + assert "[Verification Feedback]" in result + assert "Do not call complete_goal" in result + assert "test_outputs.py::test_answer" in result + + def test_exec_accepts_supported_shell_parameter(tmp_path): async def run() -> str: tool = ExecTool(working_dir=str(tmp_path), timeout=5)