From b23c6d40b3b8b99a876a0de6b778872f92893c31 Mon Sep 17 00:00:00 2001 From: Alexandre Teixeira <111787685+alteixeira20@users.noreply.github.com> Date: Sat, 3 Oct 2026 00:58:32 +0100 Subject: [PATCH] fix(effects): require evidence for external completion claims Effect obligations were consulted only for declared artifacts, and reported external success could be presented as done. Now, regardless of declared artifacts: - the latest effect on any changed file contradicted by a fresh readback fails the run (a superseded earlier effect is history, not a contradiction); - a passing verifier followed by an effect that may have changed state without settled evidence is stale (BLOCKED); - executed external effects that are not VERIFIED cap the decision at UNVERIFIED, and the answer always carries server-authored facts for them ("reported success; any external change it made was not independently verified", "reported failure", "unknown outcome"). The disclosure is structural and does not depend on recognizing the model's wording. When it is the only change, the model's answer events are released unchanged and the disclosure follows as one delta (and in round_texts). Prose filtering is also tightened (remote verbs are mutation claims, an unnamed "I updated it" cannot borrow the single required artifact, bare "Done." is a terminal claim beside unverified external effects). A passing verifier still supports test claims; it never speaks for the external effect. Replaces the uncommitted attempt that blocked every run with any RUNNING effect: a background launch with no declared obligations completes UNVERIFIED. --- src/agent_evidence.py | 125 ++++++++++++++++++++++++---- src/agent_runtime/completion.py | 51 ++++++++++-- src/agent_runtime/journal.py | 2 +- tests/test_agent_runtime_context.py | 2 + 4 files changed, 153 insertions(+), 27 deletions(-) diff --git a/src/agent_evidence.py b/src/agent_evidence.py index 29daebebe..b3c7e2b22 100644 --- a/src/agent_evidence.py +++ b/src/agent_evidence.py @@ -111,6 +111,9 @@ class EvidenceEvent: return data +EXTERNAL_EFFECT_UNVERIFIED = "an external operation's resulting state was not independently verified" + + @dataclass(frozen=True) class CompletionDecision: status: CompletionStatus @@ -675,8 +678,9 @@ class EvidenceLedger: later.append((entry, explicit)) return later - def _effect_unsettled(self, required: str) -> bool: - """A later operation may have partially changed this artifact. + @staticmethod + def _entry_unsettled(entry: Mapping[str, Any], explicit: bool) -> bool: + """One effect may have changed state with no settled evidence. Explicit targets are unsettled by unknown/timed-out/cancelled outcomes and by failures after the producer reached its mutation stage (atomic @@ -686,19 +690,68 @@ class EvidenceLedger: are already tracked through artifact version capture. """ from src.agent_runtime.effects import CleanupState, ExecutionOutcome - unknown = {ExecutionOutcome.ATTEMPTED, ExecutionOutcome.INTERRUPTED, ExecutionOutcome.CANCELLED} - for entry, explicit in self._later_effects(required): - assessment = entry["assessment"] - if not assessment.unresolved_impact: - continue - if assessment.execution in unknown or assessment.execution is ExecutionOutcome.RUNNING: - return True - if explicit and (assessment.execution is ExecutionOutcome.TIMED_OUT - or (assessment.execution is ExecutionOutcome.FAILED and entry.get("mutation_attempted"))): - return True - if not explicit and assessment.cleanup is CleanupState.FAILED: - return True - return False + unknown = {ExecutionOutcome.ATTEMPTED, ExecutionOutcome.INTERRUPTED, ExecutionOutcome.CANCELLED, + ExecutionOutcome.RUNNING} + assessment = entry["assessment"] + if not assessment.unresolved_impact: + return False + if assessment.execution in unknown: + return True + if explicit: + return (assessment.execution is ExecutionOutcome.TIMED_OUT + or (assessment.execution is ExecutionOutcome.FAILED and bool(entry.get("mutation_attempted")))) + return assessment.cleanup is CleanupState.FAILED + + def _effect_unsettled(self, required: str) -> bool: + """A later operation may have partially changed this artifact.""" + return any(self._entry_unsettled(entry, explicit) for entry, explicit in self._later_effects(required)) + + def _current_effects(self) -> list[dict[str, Any]]: + """Effects of this journal's own actions, in action order.""" + return sorted((entry for entry in self.effects if type(entry.get("ordinal")) is int), + key=lambda entry: entry["ordinal"]) + + def _contradicted_target(self) -> str: + """A changed file whose latest effect a fresh readback contradicts. + + Only the latest effect per target counts: an earlier effect superseded + by a later requested write is history, not a contradiction. + """ + from src.agent_runtime.effects import EffectVerdict + latest: dict[str, dict[str, Any]] = {} + for entry in self._current_effects(): + for path in entry.get("paths") or (): + latest[path] = entry + return next((path for path, entry in latest.items() + if entry["assessment"].verdict is EffectVerdict.CONTRADICTED), "") + + def unverified_external_effects(self) -> list[dict[str, Any]]: + """Executed external effects whose resulting state is not verified. + + A remote acknowledgement is execution evidence only. Without an + admitted independent readback these effects never support a + definitive statement that the external state changed. + """ + from src.agent_runtime.effects import EffectVerdict + return [entry for entry in self.effects if entry.get("external") + and entry["assessment"].verdict not in {EffectVerdict.VERIFIED, EffectVerdict.NOT_EXECUTED}] + + def effect_disclosures(self) -> tuple[str, ...]: + """Server-authored facts for unverified external effects.""" + from src.agent_runtime.effects import ExecutionOutcome + facts = [] + for entry in self.unverified_external_effects(): + tool = str(entry.get("tool") or "external operation") + execution = entry["assessment"].execution + if execution is ExecutionOutcome.REPORTED_SUCCESS: + facts.append(f"External operation {tool} reported success; any external change it made was " + "not independently verified.") + elif execution is ExecutionOutcome.FAILED: + facts.append(f"External operation {tool} reported failure; it may have partially taken effect.") + else: + facts.append(f"External operation {tool} has an unknown outcome; it may or may not have " + "taken effect.") + return tuple(dict.fromkeys(facts)) def _effect_contradicted(self, required: str) -> str: """The latest effect targeting the artifact, if fresh readback contradicts it.""" @@ -873,8 +926,12 @@ class EvidenceLedger: ) def _supports_verifier_claim(self, identities: Sequence[str] = (), paths: Sequence[str] = ()) -> bool: - """Only the current passing verifier may support its named runner.""" - if self.evaluate().status != CompletionStatus.VERIFIED: + """Only the current passing verifier may support its named runner. + + A test result stays a test result when an unrelated external effect + keeps the whole run unverified; it never speaks for that effect. + """ + if self._evaluate_obligations().status != CompletionStatus.VERIFIED: return False latest = next((event for event in reversed(self.events) if event.kind == EvidenceKind.VERIFIER_RESULT and event.authoritative), None) @@ -895,6 +952,9 @@ class EvidenceLedger: targets = tuple(paths) or self.requirements.required_artifacts if not targets or (not paths and len(targets) != 1): return False + if not paths and self.unverified_external_effects(): + # An unnamed "I updated it" may mean the external effect. + return False for path in targets: matching = [event for event in self.events if event.kind == kind and event.authoritative and _artifact_path_matches_required(event.artifact_path, path, self.requirements.workspace_root)] @@ -935,6 +995,21 @@ class EvidenceLedger: *, exhausted: bool = False, awaiting_user: bool = False, + ) -> CompletionDecision: + decision = self._evaluate_obligations(exhausted=exhausted, awaiting_user=awaiting_user) + if decision.status in {CompletionStatus.VERIFIED, CompletionStatus.SATISFIED} and \ + self.unverified_external_effects(): + # Reported external execution is not a verified effect: the run + # may end, but never as verified or satisfied. + return CompletionDecision(CompletionStatus.UNVERIFIED, True, EXTERNAL_EFFECT_UNVERIFIED, + decision.evidence_ids, decision.missing_artifacts) + return decision + + def _evaluate_obligations( + self, + *, + exhausted: bool = False, + awaiting_user: bool = False, ) -> CompletionDecision: if awaiting_user: return CompletionDecision( @@ -984,6 +1059,22 @@ class EvidenceLedger: "fresh readback contradicts the requested artifact content", (), (required,)) + if self.effects: + # Effect obligations hold whether or not artifacts were declared. + contradicted = self._contradicted_target() + if contradicted: + return CompletionDecision(CompletionStatus.FAILED, False, + "fresh readback contradicts the requested state of a changed file", + (), (contradicted,)) + if latest_verifier is not None: + floor = self._action_order.get(latest_verifier.action_id, 0) + if any(entry["ordinal"] > floor and self._entry_unsettled(entry, bool(entry.get("paths"))) + for entry in self._current_effects()): + return CompletionDecision( + CompletionStatus.BLOCKED, False, + "a later operation may have changed state after the latest executable verifier", + (latest_verifier.event_id,)) + satisfied_ids: list[str] = [] missing: list[str] = [] unsettled: list[str] = [] diff --git a/src/agent_runtime/completion.py b/src/agent_runtime/completion.py index 943074df3..6bf9ef374 100644 --- a/src/agent_runtime/completion.py +++ b/src/agent_runtime/completion.py @@ -42,9 +42,10 @@ _TEST_STATUS_CLAIM = re.compile( r'(?:pass(?:ed|ing)?|succeeded|successful(?:ly)?|green)\b|' r'\b(?:zero|no|0)\s+(?:test\s+)?failures\b', re.I) _EXECUTION_CLAIM = re.compile( - r'\b(?:(?:I|we|I\'ve|we\'ve|and)\s+(?:have\s+)?(?:successfully\s+)?(?:ran|executed|tested|verified|created|updated|modified|wrote|saved|fixed|completed)|' - rf'(?:file|artifact|command|script|service|server|{_ARTIFACT_PATH})\s+(?:was\s+|has\s+been\s+|is\s+)?(?:successfully\s+)?(?:created|updated|written|saved|executed|started)|' - r'(?:successfully\s+)(?:ran|executed|created|updated|saved|completed))\b', re.I) + r'\b(?:(?:I|we|I\'ve|we\'ve|and)\s+(?:have\s+)?(?:successfully\s+)?(?:ran|executed|tested|verified|created|updated|modified|wrote|saved|fixed|completed|sent|deleted|submitted|published|deployed|configured|uploaded)|' + rf'(?:file|artifact|command|script|service|server|email|message|record|resource|{_ARTIFACT_PATH})\s+(?:was\s+|has\s+been\s+|is\s+)?(?:successfully\s+)?(?:created|updated|written|saved|executed|started|sent|deleted|submitted|published|deployed|configured)|' + r'(?:successfully\s+)(?:ran|executed|created|updated|saved|completed|sent|deleted|submitted|published|deployed)|' + r'(?:the\s+)?(?:remote\s+)?(?:operation|request|call|mutation|action)\s+(?:was\s+|has\s+)?(?:successfully\s+)?(?:completed|succeeded|finished))\b', re.I) _UNATTESTED_TEST_METRIC = re.compile( r'\b\d+\s+(?:(?:unit|integration)\s+)?tests?\s+pass(?:ed|ing)?\b|' r'\b\d+\s+passed\b|\b\d+(?:\.\d+)?%\s+(?:test\s+)?coverage\b', re.I) @@ -52,7 +53,7 @@ _UNBOUNDED_SUCCESS = re.compile( r'\b(?:everything|all\s+(?:bugs|issues))\s+(?:is\s+|are\s+|has\s+been\s+)?' r'(?:fixed|resolved|working)\b', re.I) _MUTATION_CLAIM = re.compile( - r'\b(?:created|updated|modified|wrote|written|saved|fixed)\b', re.I) + r'\b(?:created|updated|modified|wrote|written|saved|fixed|sent|deleted|submitted|published|deployed|configured|uploaded)\b', re.I) _TEST_IDENTITY = re.compile(r'\b(?:pytest|unittest)\b', re.I) _TEST_SUBJECT = re.compile(r'\b(?:tests?|test suite|pytest|unittest|checks?|verification)\b', re.I) _CLAIM_PATH = re.compile(_ARTIFACT_PATH) @@ -106,17 +107,20 @@ def _supported_prose(text: str, ledger: EvidenceLedger, decision: CompletionDeci """Remove unsupported assertions at statement boundaries; add no notice.""" incomplete = decision.reason if not decision.can_complete and decision.status != CompletionStatus.AWAITING_USER else '' execution_required = _execution_obligation(ledger.requirements) + # Bare "Done." cannot stand for an external effect nobody verified. + terminal_claims = execution_required or bool(ledger.unverified_external_effects()) kept = [] removed = '' for statement, scoped in _unquoted_statements(text): why = '' - for claim, scope in _current_run_claims(scoped, execution_required=execution_required): + for claim, scope in _current_run_claims(scoped, execution_required=terminal_claims): paths = tuple(match.group().rstrip('.') for match in _CLAIM_PATH.finditer(scope)) if claim == 'metric': why = 'test counts, coverage or exhaustive correctness were not established by execution evidence' elif claim == 'test': identities = tuple(match.group().lower() for match in _TEST_IDENTITY.finditer(scope)) - if decision.status != CompletionStatus.VERIFIED or not ledger._supports_verifier_claim(identities, paths): + if (decision.status not in {CompletionStatus.VERIFIED, CompletionStatus.UNVERIFIED} + or not ledger._supports_verifier_claim(identities, paths)): why = 'no current passing executable verification supports the claim' elif claim == 'mutation': if not ledger._supports_artifact_claim(EvidenceKind.ARTIFACT_MUTATION, paths): @@ -142,7 +146,22 @@ def completion_answer(text: str, ledger: EvidenceLedger, decision: CompletionDec Exit status proves neither test counts nor coverage. A bad assertion is removed at statement boundaries instead of erasing an entire explanation. The execution outcome remains separate from a discarded model assertion. + Unverified external effects are always stated by the server, so no + surviving prose can present a reported remote success as a verified one. """ + answer, reason = _completion_answer(text, ledger, decision) + return _disclose(answer, ledger), reason + + +def _disclose(answer: str, ledger: EvidenceLedger) -> str: + """Append the server's facts for unverified external effects.""" + summary = ' '.join(ledger.effect_disclosures()) + if not summary: + return answer + return (answer.rstrip() + '\n\n' + summary) if answer.strip() else summary + + +def _completion_answer(text: str, ledger: EvidenceLedger, decision: CompletionDecision) -> tuple[str, str]: incomplete = decision.reason if not decision.can_complete and decision.status != CompletionStatus.AWAITING_USER else '' execution_required = _execution_obligation(ledger.requirements) prose, removed = _supported_prose(text, ledger, decision) @@ -163,7 +182,7 @@ def completion_answer(text: str, ledger: EvidenceLedger, decision: CompletionDec facts = [] if ledger.requirements.required_artifacts: facts.append('Output available: ' + ', '.join(ledger.requirements.required_artifacts) + '.') - if decision.status == CompletionStatus.VERIFIED: + if decision.status == CompletionStatus.VERIFIED or ledger._supports_verifier_claim(): facts.append('The latest executable verification passed.') elif any(e.kind == EvidenceKind.ARTIFACT_VALIDATION and e.authoritative and e.success for e in ledger.events): facts.append('Artifact readback verified. No passing executable test result was recorded.') @@ -312,7 +331,8 @@ def with_completion_gate(func): # Exhaustion limits execution; factual source synthesis can remain # useful and must not be replaced merely because the budget ended. presentation_decision = ledger.evaluate(awaiting_user=awaiting) if exhausted and not provider_error else decision - safe_answer, reason = completion_answer(answer, ledger, presentation_decision) + filtered_answer, reason = _completion_answer(answer, ledger, presentation_decision) + safe_answer = _disclose(filtered_answer, ledger) # Evaluate each earlier draft as well as the final replacement. # Never replay an unsupported intermediate success claim. draft = ''.join(str(e.get('delta') or e.get('content') or '') @@ -328,7 +348,14 @@ def with_completion_gate(func): released_at = perf_counter() if not provider_error: yield _event({'type': 'completion_decision', 'data': decision.to_dict()}) - replaced_answer = bool(presentation_replaced or reason or unsafe_draft or safe_answer != answer) + # When the only change is the server's effect disclosure, the + # model's answer events are released unchanged and the disclosure + # follows them, so no earlier-round text is dropped. + disclosure = safe_answer[len(filtered_answer):] if safe_answer != filtered_answer else '' + disclosure_only = bool(disclosure) and not (presentation_replaced or reason or unsafe_draft + or filtered_answer != answer) + replaced_answer = not disclosure_only and bool( + presentation_replaced or reason or unsafe_draft or safe_answer != answer) if replaced_answer: reasoning = [event for event in answer_events if event.get('thinking') is True] _, unsafe_reasoning = completion_answer( @@ -341,6 +368,8 @@ def with_completion_gate(func): else: for event in answer_events: yield _event(event) + if disclosure_only: + yield _event({'delta': disclosure}) if provider_error: yield _event({'type': 'completion_decision', 'data': decision.to_dict()}) for event in metrics_events: @@ -360,6 +389,10 @@ def with_completion_gate(func): if not provider_error: metadata['round_texts'] = [safe_answer] metadata['completion_gate_reason'] = reason or unsafe_draft or 'receipt_summary' + elif disclosure_only and metadata.get('round_texts') and isinstance(metadata['round_texts'], list) \ + and isinstance(metadata['round_texts'][-1], str): + # Reload renders round_texts: keep the disclosure with them. + metadata['round_texts'] = [*metadata['round_texts'][:-1], metadata['round_texts'][-1] + disclosure] if provider_error and isinstance(metadata.get('round_texts'), list): # Failed rounds stay as per-round diagnostics, but they are # rendered again on reload. Apply the same statement filter diff --git a/src/agent_runtime/journal.py b/src/agent_runtime/journal.py index 200718bf5..b8b2edfef 100644 --- a/src/agent_runtime/journal.py +++ b/src/agent_runtime/journal.py @@ -90,7 +90,7 @@ class ActionJournal: outcome = history.latest_outcome(assessment.effect_id) entries.append({ 'ordinal': order.get(assessment.action_id), 'assessment': assessment, - 'tool': claim.operation.tool, 'unknown_scope': claim.unknown_scope, + 'tool': claim.operation.tool, 'unknown_scope': claim.unknown_scope, 'external': claim.external, 'paths': tuple(ref.location[-1] for ref in claim.impact_scope if ref.kind.value == 'filesystem'), 'mutation_attempted': bool(outcome and outcome.facts.mutation_attempted), 'artifact_changes': changes.get(assessment.action_id), diff --git a/tests/test_agent_runtime_context.py b/tests/test_agent_runtime_context.py index b6fe40562..403835231 100644 --- a/tests/test_agent_runtime_context.py +++ b/tests/test_agent_runtime_context.py @@ -1228,6 +1228,8 @@ def test_native_host_shell_call_runs_through_bridge_and_threads_result(monkeypat assert host_output["call_id"] == "call_host_1" assert host_output["tool_call_id"] == "call_host_1" assert any("ajax is at 192.168.1.42" in event.get("delta", "") for event in events) + # The host bridge is an external effect: its disclosure follows the answer. + assert any("External operation host_shell reported success" in event.get("delta", "") for event in events) def test_workspace_agents_md_lands_in_untrusted_prompt_message(tmp_path, monkeypatch):