mirror of
https://github.com/razzant/ouroboros.git
synced 2026-10-03 04:07:04 +00:00
fix(review): preserve verdict meaning and observed session actors
Remove the contributor severity-demotion heuristic in an explicit follow-up. Validate advisory rows through the existing extraction seam and read model, harness and account from one final attempt. Keep replay of a previously recorded session charge idempotent when model observation changes; preserve all ownership checks and the historical ledger bytes. Co-authored-by: Ouroboros <311266734+ouroboros-agent@users.noreply.github.com>
This commit is contained in:
Ouroboros
parent
8c352bb3fb
commit
77f4061433
27 changed files with 752 additions and 1043 deletions
|
|
@ -228,7 +228,6 @@ server.py (Starlette+uvicorn) ← HTTP + WebSocket on configurable host:port (de
|
|||
├── review_execution.py ← The ONE review-delivery seam below the substrate: closed route vocabulary with THE delivery-class predicate beside it (`delivery_retrieves(route, subagent_id)`: a hosted session or a configured-subagent api row retrieves the subject itself and never receives the packet — `ReviewSlot.retrieves`, `ConfiguredReviewerSlot.retrieves`, admission, packet fit and the surfaces' request builders all call this one definition), immutable `ReviewAssignment` bound once via `_review_route_executor`, the single physical seam `_execute_slot_attempt`, typed `ReviewAttemptResult`; no cross-transport fallback — a route that cannot deliver raises on its own slot (`ReviewRouteUnavailable`), and the durable prompt record and both physical sends share one byte-identical rendering; `ApiChatReviewExecutor` renders lazily and memoizes (digest-pinned); `AgentSessionReviewExecutor` runs one delegated read-only session — `outputSchema` is sent only when the route's own live manifest declares it and trusted only on `outputConformance == "passed"`, else strict parsing then light-model extraction disclosed as `capability_delta`; `review_output_contract(request)` is the ONE governance text every delivery honours (the api pack renders it into its byte-stable segment; a surface hands the same text to its retrieving rows as `policy["output_contract"]`), and `ROUTE_OWNED_POLICY_KEYS` (`output_contract`, `native_data_root`) are consumed by the retrieving executors and never rendered into the api pack's Policy JSON; per-row delivery via `OUROBOROS_REVIEW_ROUTES`/`OUROBOROS_SCOPE_REVIEW_ROUTES`, session target `OUROBOROS_REVIEW_SESSION_ROUTE` falling back to `OUROBOROS_SUBAGENT_HARNESS`; task acceptance follows its configured rows like every surface (owner R2 — the former `api_chat` pin is gone). Disclosed residual (pre-existing release behaviour, b9bcc2da; issue #588; out of this change's scope): a compatibility transport that raises with positive physical capture invokes the paid stamp after the send; if the tree's last paid cycle is consumed concurrently at that late stamp, the wallet refusal replaces the captured exception and the substrate may resend
|
||||
├── review_native_episode.py ← Bounded native tool-round delivery (`NativeToolRoundReviewExecutor`) for api_chat reviewer rows that reference a configured subagent (advisory included): ONE logical episode of `chat(tools=…)` calls against a fresh instance-local read-only registry (network/web off) until the reviewer answers — no round cap (P13); the bounds are the transcript bound derived from the reviewer's own window (`review_native_transcript_bound`, never above the owner ceiling `OUROBOROS_REVIEW_NATIVE_MAX_TRANSCRIPT_CHARS` — except that a surface-declared mandatory reading, `policy["native_mandatory_read_chars"]`, is a floor lifting it to `native_mandatory_read_bound` up to the window's own capacity, recorded as `native_mandatory_read_chars` and, when even that cannot hold it, the typed `native_mandatory_read_disclosure` = `native_mandatory_read_exceeds_bound`), the owner deadline (`deadline_at`, passed by every surface — commit triad, scope, advisory, task acceptance (owner R23)) together with the coordinator's logical window for the slot, and the paid ledger; the data plane is opt-in per surface (`policy["native_data_root"]`: an empty scratch by default, the caller's real root when named — never removed by the episode); paid-stamped sends; ≤200 tool receipts (an executed `read_file` receipt also carries the DELIVERED extent — `start_line`/`end_line`/`total_lines`/`eof` plus `opened_path`/`opened_root`, the root-relative path and normalized root the reader actually opened — from the reader's own `ctx.last_read_view` stamp (`tools/core.py::_read_file`), bound to ONE call structurally and cut to the complete lines the result bound delivered; the extent contract and its edge rules are `_read_extent`'s docstring, and the stamp's three-writer invariant is pinned by `tests/test_native_tool_round_executor.py`) + `host_observed` attestation + the episode facts (`native_rounds`, `native_tool_calls`, `native_transcript_chars` (the wire size of the last physical send) / `_bound` / `_refused_chars` (on a bound end: what the refused next send would have carried), `native_landing_notified` (posted to the transcript) / `native_landing_sent` (a physical send carried it), `native_end_reason`, `native_custody_row`, `native_tool_receipts[].outcome` ∈ executed | refused | error | withheld, and, on every non-delivering or incomplete end that reached an assistant round — bound, deadline, ledger or transport, the landing notice last or not — a bounded, structurally valid, redacted `native_terminal_round` document) on the actor usage plus one typed custody row (`review_native_episode`) per episode end; exhaustion is a typed `native_transcript_cap_exceeded` refusal for verdict shapes and a disclosed `native_incomplete` product for the report shape; a second actor attempt is LOCAL format repair over the collected answer, never a second paid episode; the normative episode contract (landing notice, floors, pre-send refusals, typed ends, custody) is stated ONCE under §6 Review delivery
|
||||
├── review_verdict_extraction.py ← Session/native verdict canonicalization: strict parse first, then light-model extraction to the review's own contract; branches on the surface's output SHAPE (`triad_review.review_output_shape`): `array` keeps the historical findings ladder; `object` (task acceptance) keeps the WHOLE verdict object on the schema, strict and extraction branches (an acceptance object is never reduced to its findings list, and `[]` is not a strict object verdict); `report` (deep self-review) passes the product through verbatim, never extracted
|
||||
├── review_cross_check.py ← Reviewer-verdict cross-check: `_cross_check_findings` downgrades a `critical` finding whose identifier/path claim ("imports X", "calls Y") cannot be substantiated against actual repo content to `advisory` with an audit note. Pure, read-only, git-backed, bounded walk (`_WALK_BUDGET_SEC` / `_WALK_FILE_CAP`), fails open. Invoked from `canonicalize_session_verdict` only when the caller supplies `repo_root`; `review_execution` re-exports it for tests
|
||||
├── review_session_custody.py ← Exact delegated-review recovery validation + pre-POST durable invocation checkpoint; no scheduler or state store
|
||||
├── review_slot_cancel.py ← Slot-cancel honesty: a cancel outcome reports only what it PROVED ("host-cancelled" only on a confirmed own-state receipt; confirmed failed/interrupted is attributed to the run's own terminal); a succeeded run whose result read fails is typed `ReviewSessionSucceededResultUnavailable` after one bounded retry, never "may still be live"
|
||||
├── review_actor_aggregation.py ← Contract aggregation for completed review actor rows; demotes non-contract-valid responses
|
||||
|
|
@ -1101,12 +1100,45 @@ Review delivery has two closed route kinds in `review_execution.py`: `api_chat`
|
|||
|
||||
Advisory availability is evaluated from the current configured slot and route, never inferred from a stale stored verdict. A disabled advisory slot is an audited bypass; an `api_chat` row requires provider credentials for its RESOLVED model, an `agent_session` row a resolvable session route. If the commit advisory is unavailable, the commit gate runs its compensating hermetic preflight only when tests remain independently applicable (not explicitly skipped, diff not documentation-only). Malformed structured slot configuration is refused at save and becomes a typed loud review-time failure for commit triad, scope, advisory, plan, skill review — and deep self-review (`deep_review_slot()` raises on the malformed value and `run_deep_self_review` returns the typed `deep_self_review_unavailable` result instead of a report). Task acceptance refuses the same way (a typed DEGRADED panel, `reviewer_slot_config_invalid`; owner R3). No surface silently chooses the opposite route or a default panel.
|
||||
|
||||
Session reviewer identity comes from `gateways.claudexor.final_attempt_facts`: the
|
||||
unique `final_attempt_id` row in the engine-owned `final/telemetry.yaml`, bound to
|
||||
the requested run id. Model, harness and credential profile come from that SAME
|
||||
attempt. `summary.model`/`harnesses` echo requests, and summary route/auth projections
|
||||
may borrow earlier-attempt facts; none supplies a missing observation. Reviewer
|
||||
usage, last-execution views, delegated terminal payloads and settlement preserve
|
||||
known facts or explicit absence. The original custody/billing route remains its
|
||||
chosen authority, separate from the observed actor. No new quorum rule or model-name
|
||||
mapping is implied by this disclosure.
|
||||
|
||||
Advisory row parsing owns its `PASS|FAIL` and `critical|advisory` values at
|
||||
`preflight_review_run._is_checklist_array`. Case and surrounding whitespace are
|
||||
canonicalized once for every consumer. An unknown verdict or unknown/missing FAIL
|
||||
severity rejects the whole array into the existing bounded extraction rail;
|
||||
unresolved output stays `parse_failure` with its full source retained. PASS without
|
||||
a severity remains compatible, and the separate genuine-empty-clean predicate is
|
||||
unchanged. An optional array validator lets the shared canonicalizer honor this
|
||||
surface contract without changing triad, object-verdict or report semantics.
|
||||
Canonicalization never changes the reviewer's judgment by searching the repository
|
||||
for words or identifiers.
|
||||
|
||||
### Usage ledger substrate vs. accounting policy
|
||||
|
||||
`usage_ledger.py` owns the durable append-only physical-attempt ledger (cross-process locking, sequence and transition validation, append+fsync, replay, loud tail quarantine); `usage_accounting.py` is the one-way policy layer above it (pricing, reservations, settlement, scopes, budget fences, imports, projections, admission), and the substrate never imports policy. The boundary means a pricing or budget-policy change cannot redefine valid ledger storage, and a locking or repair change cannot silently change what an attempt costs; compatibility events, state mirrors, task fields, and UI projections may carry attempt ids and derived totals but never become a second charge source.
|
||||
|
||||
### Delegated subagents (Claudexor transport + the nanny)
|
||||
|
||||
Ordinary delegation requests no extra engine review panel; new ordinary runs on
|
||||
the pinned Claudexor 3.9.8 default to no panel. The start receipt's `engine_version`
|
||||
is the handshaken serving version, distinct from the release pin. Older serving
|
||||
engines and already-recorded runs can retain historical review behavior. Engine
|
||||
review outcome, execution success, the parent's integration decision and Ouroboros
|
||||
review gates remain separate. Timeline projection retains each known participant's
|
||||
`harnessId`/`attemptId`, including non-text reviewer events, and names them on live
|
||||
progress without guessing absent identities. Delegated snapshot capture remains
|
||||
relative to its recorded baseline: committed bytes can be captured with a disclosed
|
||||
`head_moved`; the instruction still forbids committing, and the distinct
|
||||
`self_worktree` unchanged-HEAD check is preserved.
|
||||
|
||||
Children coordinate through `tree_note` and `tree_read`; only the parent may use `override_delegation_constraint`, and a `review_requested` note carries an exact evidence reference/hash and wakes the parent without starting a paid cycle. Both read-only and acting children hold the descendant-scoped `forward_to_worker`, `peek_task`, `cancel_task`, and `discard_child_result` controls; recursive delegation never widens filesystem, budget, depth, deadline, commit, or owner authority. `delegation_budget` governs descendants (`may_delegate`, `may_fan_out`, additive depth provenance; a free-form intent note is never authority); persisted admission facts outrank later Settings changes, and a lower permitted depth is reported `capability_reduced`, never a silent flat tree.
|
||||
|
||||
**Registry.** `OUROBOROS_SUBAGENTS` is the active task-actor SSOT: a strict `{enabled, items}` value of at most ten `ConfiguredSubagent` rows — stable `subagent_id`, owner-authored English `recommended_use`, one normalized route (`api_model` or `agent_session`), optional effort and session credential pin (`configured_subagents.py`; legacy env keys are fail-closed migration inputs). The description is selection context only — host code never parses, ranks, or maps its words to task text — so API models and session harnesses occupy one LLM-selectable list without pretending they share a topology; an exact owner selection either starts that route or reports why not, never a keyword router or automatic API substitution.
|
||||
|
|
|
|||
|
|
@ -949,6 +949,18 @@ canonicalizer: the shape table is form only, and a new object- or
|
|||
report-shaped surface registers there instead of teaching the extraction rail
|
||||
another `if`.
|
||||
|
||||
Advisory validates its own row enums through the shared canonicalizer's optional
|
||||
array validator. Unknown verdicts or unknown/missing FAIL severity remain unparsed
|
||||
unless the existing extraction can faithfully recover them; neither path invents
|
||||
critical severity or downgrades a finding from identifier presence. Preserve the
|
||||
full raw result, ordinary PASS rows and genuine empty-clean responses in tests.
|
||||
Hosted-review model/harness/profile evidence comes from the same final attempt in
|
||||
`final/telemetry.yaml`, not requested values or cross-attempt summary projections.
|
||||
Missing observations remain unknown; the exact contributor checker still refuses
|
||||
unconfirmed model identity, including a display label that cannot prove the pin.
|
||||
Ordinary delegation requests no extra engine panel; the start receipt names the
|
||||
serving engine version, and historical runs can retain older review behavior.
|
||||
|
||||
Paid review cycles across the gates are bounded by one shared owner knob,
|
||||
`OUROBOROS_REVIEW_MAX_CYCLES` — a STRING, positive integer or `unlimited`,
|
||||
default `"2"` (Settings → Behavior → "Max Review Cycles"). Its SSOT is
|
||||
|
|
|
|||
|
|
@ -40,6 +40,14 @@ validated aggregate plus every row that is still live.
|
|||
|
||||
## 4. Baseline block shape
|
||||
|
||||
Subscription-session replay binds the stable external session id, route and all
|
||||
ownership/review-attribution fields. Its observed model is disclosure, not a new
|
||||
physical session or a pricing input: a later observation can differ or become
|
||||
unknown while replay returns the original ledger row byte-identically. Existing
|
||||
legacy model labels are historical evidence, not newly confirmed observations;
|
||||
new custody/result observations remain separate. External-unmetered replay keeps
|
||||
its own existing model identity check.
|
||||
|
||||
The compacted file is `[header row] [group rows …] [retained rows …]`.
|
||||
|
||||
**Header** (`kind="usage_baseline"`, exactly one, always seq 1 when present):
|
||||
|
|
|
|||
|
|
@ -119,7 +119,7 @@ class RunCustody:
|
|||
task_id: str = ""
|
||||
route_id: str = ""
|
||||
model: str = ""
|
||||
# Requested pin (`credentialProfileId`) from the start body; '' = automatic; applied half = settlement authRoute.
|
||||
# Requested pin (`credentialProfileId`); '' = automatic; applied half = final-attempt telemetry.
|
||||
profile_id: str = ""
|
||||
project_id: str = ""
|
||||
project_owned: bool = False
|
||||
|
|
@ -915,17 +915,18 @@ def settle_run(drive_root: Any, gateway: Any, custody: RunCustody, detail: Dict[
|
|||
return {"settled": True, "ledger_recorded": True,
|
||||
"project_retired": not custody.project_owned and not custody.project_persistent,
|
||||
"project_persistent": custody.project_persistent, "retried": False}
|
||||
from ouroboros.gateways.claudexor import final_attempt_facts
|
||||
|
||||
summary = summary_of(detail)
|
||||
observed = final_attempt_facts(detail, custody.run_id)
|
||||
# Claudexor reports CASH in `spendUsd`, EXACTNESS in `spendEstimated`. A run
|
||||
# is only free when the amount is really zero AND really settled: expired
|
||||
# sessions, bill-by-construction routes and auth fallbacks all charge, and
|
||||
# writing 0.0/cost_final=True over them hides money from every budget fence.
|
||||
spend, estimated = disclosed_spend(summary)
|
||||
# D29: the applied credential-profile id + access profile the deciding
|
||||
# attempt disclosed (authRoute receipt / effectiveAccess), written to the
|
||||
# durable row by default. Null on runs whose engine telemetry predates the
|
||||
# receipt — empty string, never invented.
|
||||
applied_profile = str((summary.get("authRoute") or {}).get("profileId") or "")
|
||||
# Model and credential profile belong to one final attempt. The run-level
|
||||
# authRoute can borrow an earlier account; missing final facts stay unknown.
|
||||
applied_profile = observed.get("profile_id", "")
|
||||
# Only `effectiveAccess` testifies. The daemon computes `access` as
|
||||
# `effectiveAccess ?? the client's own parsed request`, so falling back to it wrote
|
||||
# our own ASK into the durable row under a column that promises applied facts.
|
||||
|
|
@ -938,7 +939,7 @@ def settle_run(drive_root: Any, gateway: Any, custody: RunCustody, detail: Dict[
|
|||
custody.run_id,
|
||||
drive_root=pathlib.Path(custody.ledger_root or drive_root),
|
||||
route=custody.route_id,
|
||||
model=str(summary.get("model") or ""),
|
||||
model=observed.get("model", ""),
|
||||
task_id=custody.task_id,
|
||||
root_task_id=custody.root_task_id,
|
||||
parent_task_id=custody.parent_task_id,
|
||||
|
|
@ -969,10 +970,11 @@ def settle_run(drive_root: Any, gateway: Any, custody: RunCustody, detail: Dict[
|
|||
"run_id": custody.run_id,
|
||||
"task_id": custody.task_id,
|
||||
"route": custody.route_id,
|
||||
# The ENGINE-reported model (the STARTED row carries only the requested
|
||||
# pin, which is usually empty) — so execution evidence can name what
|
||||
# the harness really ran without joining to the ledger.
|
||||
"model": str(summary.get("model") or ""),
|
||||
# Route above remains custody authority. Fresh observations
|
||||
# may differ from a replayed historical ledger row's model;
|
||||
# they never rewrite that row, ownership, bounds or spend.
|
||||
"model": observed.get("model", ""),
|
||||
"observed_attempt": observed,
|
||||
"state": str(summary.get("state") or ""),
|
||||
# The SAME facts the ledger row just recorded. An undisclosed spend was emitted
|
||||
# here as `0.0` beside a flag — the render-unknown-as-zero shape the ledger row
|
||||
|
|
@ -982,9 +984,8 @@ def settle_run(drive_root: Any, gateway: Any, custody: RunCustody, detail: Dict[
|
|||
"cost_final": spend is not None and not estimated,
|
||||
"spend_disclosed": spend is not None,
|
||||
"spend_estimated": estimated,
|
||||
# D29: the applied account rides the settlement event too, so the
|
||||
# durable event stream answers "which account paid" without joining
|
||||
# to the ledger row.
|
||||
# The final attempt's account rides the settlement event too;
|
||||
# a replayed ledger can retain its older observation unchanged.
|
||||
"credential_profile_id": applied_profile,
|
||||
"access_profile": applied_access,
|
||||
})
|
||||
|
|
|
|||
|
|
@ -68,13 +68,14 @@ def _bounded(rows: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
|
|||
for row in rows[-_TIMELINE_TAIL:]:
|
||||
item = {"type": _label(row.get("type")), "title": _label(row.get("title")),
|
||||
"severity": _label(row.get("severity"))}
|
||||
for key in ("attemptId", "harnessId"):
|
||||
if isinstance(row.get(key), str) and row[key]:
|
||||
item[key] = _label(row[key])
|
||||
if row.get("textKind") in _TEXT_KINDS and isinstance(row.get("detail"), str):
|
||||
# The typed body preserves stream whitespace; the engine's title is
|
||||
# only a preview. Keep the same disclosed bound, without a second copy.
|
||||
item.update(title=_label(row["detail"]), textKind=row["textKind"],
|
||||
textDelta=row.get("textDelta") is True,
|
||||
attemptId=_label(row.get("attemptId")),
|
||||
harnessId=_label(row.get("harnessId")))
|
||||
textDelta=row.get("textDelta") is True)
|
||||
out.append(item)
|
||||
return out
|
||||
|
||||
|
|
@ -541,6 +542,10 @@ def live_line(run_id: str, advance: _Advance) -> str:
|
|||
text_kind = row.get("textKind")
|
||||
is_text = text_kind in _TEXT_KINDS and isinstance(row.get("title"), str)
|
||||
text = row["title"] if is_text else str(row.get("title") or row.get("type") or "")
|
||||
if not is_text:
|
||||
actor = "/".join(str(row[key]) for key in ("harnessId", "attemptId") if row.get(key))
|
||||
if actor:
|
||||
text = f"[{actor}] {text}"
|
||||
stream = (text_kind, row.get("attemptId"), row.get("harnessId")) \
|
||||
if is_text and row.get("textDelta") is True else None
|
||||
if not text:
|
||||
|
|
|
|||
|
|
@ -8,11 +8,14 @@ from ouroboros.delegate_shared import _fail
|
|||
|
||||
|
||||
HOST_INSTRUCTIONS = (
|
||||
"You are a delegated worker running inside another agent's working tree. Your "
|
||||
"You are a delegated worker running inside the workspace assigned by your host. Your "
|
||||
"authority is everything INSIDE this root and nothing outside it. Do not run git "
|
||||
"commit, tag, push, rebase, reset or any other history-moving command: your host "
|
||||
"takes the diff of this tree and integrates it itself, and a moved HEAD invalidates "
|
||||
"that diff and destroys your work. Do not review or accept your own change, do not "
|
||||
"captures changes against its recorded baseline and decides whether to integrate "
|
||||
"them. A private delegated snapshot can preserve committed changes in that diff, "
|
||||
"but a moved HEAD is disclosed as an instruction violation; it does not authorize "
|
||||
"a commit or apply. A self_worktree capture separately requires an unchanged HEAD. "
|
||||
"Do not review or accept your own change, do not "
|
||||
"touch the host's runtime controls, skills, or memory, and do not write outside "
|
||||
"this root. If your environment offers a way to ask your host a clarifying "
|
||||
"question, you may use it: your host may answer from its task context; a question "
|
||||
|
|
|
|||
|
|
@ -917,6 +917,45 @@ def attempt_containment(run_dir: str) -> List[AttemptContainment]:
|
|||
return applied
|
||||
|
||||
|
||||
def final_attempt_facts(detail: Dict[str, Any], run_id: str) -> Dict[str, str]:
|
||||
"""Read the final attempt's route facts from engine-owned telemetry.
|
||||
|
||||
The summary's model and harnesses echo the request; its route/authRoute
|
||||
projections may borrow facts from earlier attempts. Only the unique row
|
||||
named by final_attempt_id belongs to the delivered result. Missing facts
|
||||
stay unknown, never filled from another attempt or the request.
|
||||
"""
|
||||
summary = detail.get("summary") if isinstance(detail, dict) else None
|
||||
if not isinstance(summary, dict) or not isinstance(run_id, str) or not run_id.strip():
|
||||
return {}
|
||||
run_dir = summary.get("runDir")
|
||||
if not isinstance(run_dir, str) or not run_dir.strip():
|
||||
return {}
|
||||
import yaml # type: ignore
|
||||
|
||||
try:
|
||||
record = yaml.safe_load(
|
||||
(pathlib.Path(run_dir) / "final" / "telemetry.yaml").read_text(encoding="utf-8"))
|
||||
except (OSError, ValueError, RuntimeError, yaml.YAMLError):
|
||||
return {}
|
||||
if not isinstance(record, dict) or record.get("run_id") != run_id:
|
||||
return {}
|
||||
final_id, attempts = record.get("final_attempt_id"), record.get("attempts")
|
||||
if not isinstance(final_id, str) or not final_id.strip() or not isinstance(attempts, list):
|
||||
return {}
|
||||
matching = [row for row in attempts if isinstance(row, dict) and row.get("attempt_id") == final_id]
|
||||
if len(matching) != 1:
|
||||
return {}
|
||||
row = matching[0]
|
||||
return {
|
||||
target: row.get(source) if isinstance(row.get(source), str) else ""
|
||||
for target, source in (
|
||||
("attempt_id", "attempt_id"), ("harness_id", "harness_id"),
|
||||
("model", "observed_model"), ("profile_id", "profile_id"),
|
||||
)
|
||||
}
|
||||
|
||||
|
||||
__all__ = [
|
||||
"AttemptContainment",
|
||||
"ClaudexorGateway",
|
||||
|
|
@ -928,6 +967,7 @@ __all__ = [
|
|||
"discover_daemon",
|
||||
"discover_daemon_at",
|
||||
"engine_at_least",
|
||||
"final_attempt_facts",
|
||||
"operator_home",
|
||||
"pending_interactions",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -1,289 +0,0 @@
|
|||
"""Reviewer-verdict cross-check: downgrade a critical finding whose factual
|
||||
claim ("imports X", "calls Y", "the removed line Z is still present") cannot be
|
||||
substantiated against actual repo content.
|
||||
|
||||
Pure, read-only, git-backed ground truth. Called from
|
||||
``canonicalize_session_verdict`` only when the caller supplies ``repo_root``;
|
||||
fails open (no downgrade) on any probe error.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import pathlib
|
||||
import re
|
||||
import time
|
||||
from typing import Any, Dict, List, Optional, Tuple
|
||||
|
||||
# The slow-path walk runs on the synchronous verdict-canonicalization path.
|
||||
# Bound it hard so a pathological tree (or a review that emits many
|
||||
# unverifiable identifiers) can never stall finalization: on hitting either
|
||||
# ceiling the walk stops and the identifier is treated as PRESENT (no
|
||||
# downgrade fabricated from incomplete evidence).
|
||||
_WALK_BUDGET_SEC = 3.0
|
||||
_WALK_FILE_CAP = 4000
|
||||
|
||||
|
||||
def _identifier_present_in_repo(identifier: str, repo_root: pathlib.Path) -> bool:
|
||||
"""True if ``identifier`` appears in source files under ``repo_root``.
|
||||
|
||||
Fast path (dotted-path module resolution): ``ouroboros.config`` →
|
||||
``repo_root/ouroboros/config.py`` (or ``config/`` package). Catches both
|
||||
the real ouroboros.project_naming (file exists) and the hallucinated
|
||||
``ouroborosproject_naming`` (no path matches because the dotted join
|
||||
fails the alnum check).
|
||||
|
||||
Slow path (bounded substring walk): walks source files (.py, .js, .ts,
|
||||
.go, .rs, .rb, .java, .c, .cpp, .h, .hpp, .md, .rst, .txt), skips hidden
|
||||
and vendor/cache trees, caps files at 1MB. Used for tokens that are not
|
||||
module paths (CamelCase class names like ``ClaudexorUnavailable``).
|
||||
|
||||
The walk is on the slow path by design: the cross-check runs once per
|
||||
reviewer verdict, on a repo that the host already owns — there is no
|
||||
exfiltration surface and no tenant boundary to defend. Errors default to
|
||||
"present" (return True) so a flaky walk never fabricates a downgrade.
|
||||
"""
|
||||
if not identifier or len(identifier) < 3:
|
||||
return True # trivially present, never claim hallucination on short tokens
|
||||
|
||||
# Fail-closed tracker: any per-file I/O uncertainty (stat/open/exists
|
||||
# raising OSError on a file we wanted to inspect) is recorded here.
|
||||
# If the walk completes without finding the identifier AND we observed
|
||||
# at least one OSError, we cannot substantiate "absent" — the caller's
|
||||
# downgrade path must treat the identifier as present to avoid
|
||||
# fabricating a downgrade from incomplete evidence. Aligns the
|
||||
# per-file catch arms with the function docstring's "Errors default
|
||||
# to 'present'" contract.
|
||||
saw_any_oserror = False
|
||||
|
||||
# Fast path 1: dotted-path module resolution. ``ouroboros.review_execution``
|
||||
# → ``repo_root/ouroboros/review_execution.py`` or
|
||||
# ``repo_root/ouroboros/review_execution/__init__.py``.
|
||||
if (
|
||||
"." in identifier
|
||||
and " " not in identifier
|
||||
and identifier.replace(".", "").replace("_", "").isalnum()
|
||||
):
|
||||
try:
|
||||
parts = identifier.split(".")
|
||||
head = repo_root.joinpath(*parts[:-1])
|
||||
tail = parts[-1]
|
||||
for candidate in (
|
||||
head / f"{tail}.py",
|
||||
head / tail / "__init__.py",
|
||||
head / tail,
|
||||
):
|
||||
try:
|
||||
if candidate.exists():
|
||||
return True
|
||||
except OSError:
|
||||
saw_any_oserror = True
|
||||
continue
|
||||
except (OSError, ValueError):
|
||||
pass
|
||||
|
||||
# Slow path: bounded substring walk over source-ish files.
|
||||
SUSPECT_EXTS = (
|
||||
".py", ".js", ".ts", ".tsx", ".go", ".rs", ".rb", ".java",
|
||||
".c", ".cpp", ".h", ".hpp", ".md", ".rst", ".txt",
|
||||
)
|
||||
SKIP_DIRS = frozenset({
|
||||
"__pycache__", "node_modules", "venv", ".venv",
|
||||
"dist", "build", "target", "site-packages",
|
||||
# runtime / non-source trees on the host repo
|
||||
"data", "state", "logs", "artifacts", "task_results",
|
||||
"observability", "coverage", "htmlcov",
|
||||
})
|
||||
deadline = time.monotonic() + _WALK_BUDGET_SEC
|
||||
scanned = 0
|
||||
try:
|
||||
for dirpath, dirnames, filenames in os.walk(repo_root):
|
||||
dirnames[:] = [
|
||||
d for d in dirnames
|
||||
if not d.startswith(".") and d not in SKIP_DIRS
|
||||
]
|
||||
for fn in filenames:
|
||||
if not fn.endswith(SUSPECT_EXTS):
|
||||
continue
|
||||
scanned += 1
|
||||
if scanned > _WALK_FILE_CAP or time.monotonic() > deadline:
|
||||
return True # bounded out: cannot substantiate "absent"
|
||||
fp = pathlib.Path(dirpath) / fn
|
||||
try:
|
||||
if fp.stat().st_size > 1_000_000:
|
||||
continue
|
||||
except OSError:
|
||||
saw_any_oserror = True
|
||||
continue
|
||||
try:
|
||||
with open(fp, "r", encoding="utf-8", errors="ignore") as fh:
|
||||
if identifier in fh.read():
|
||||
return True
|
||||
except OSError:
|
||||
saw_any_oserror = True
|
||||
continue
|
||||
except OSError:
|
||||
return True # walk failed: default to "present" to avoid false downgrade
|
||||
return saw_any_oserror
|
||||
|
||||
|
||||
# Patterns used to extract code-shaped claims from a finding's reason/item.
|
||||
# Order matters: backticks first (the official code-quoting signal), then
|
||||
# dotted-path candidates, then CamelCase class names. The CamelCase pass is
|
||||
# the catch-all that catches the ``ClaudexorUnavailable`` hallucination
|
||||
# (``Claudexor.unavailable`` is split by the dot, both halves are CamelCase;
|
||||
# ``ClaudexorUnavailable`` with the dot removed is one CamelCase token).
|
||||
_BACKTICK_RE = re.compile(r"`([^`\n]+)`")
|
||||
_DOTTED_PATH_RE = re.compile(r"\b([a-z_][a-z0-9_]*(?:\.[a-z_][a-z0-9_]*){1,})\b")
|
||||
_CAMEL_CASE_RE = re.compile(r"\b([A-Z][A-Za-z0-9_]{2,})\b")
|
||||
|
||||
# "Imports", "uses", "typo", "missing", "should be" — phrases whose presence
|
||||
# turns a vague critical into one that names a verifiable fact. The
|
||||
# cross-check stays conservative: a critical that just says "FAIL" with no
|
||||
# verifiable identifiers is LEFT ALONE; we never widen the downgrade scope
|
||||
# beyond what the reviewer asserted.
|
||||
_STRONG_CLAIM_RE = re.compile(
|
||||
r"\b(imports?|uses?\b|referenc(?:es?|ing)\b|missing|typo|"
|
||||
r"does\s+not\s+exist|doesn['\u2019]t\s+exist|"
|
||||
r"should\s+be|expected\s+to|supposed\s+to|"
|
||||
r"cannot\s+be\s+found|cannot\s+find|"
|
||||
r"undefined|undeclared|unresolved|"
|
||||
r"calls?\b|invokes?\b|invocation\s+of)\b",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
# Tokens we never treat as verifiable code identifiers (common prose).
|
||||
_NON_CODE_TOKENS = frozenset({
|
||||
"PASS", "FAIL", "OK", "TODO", "FIXME", "XXX", "NULL", "NONE",
|
||||
"PYTHON", "JSON", "YAML", "TOML", "MARKDOWN", "BASH", "SHELL",
|
||||
"GIT", "URL", "URI", "API", "CLI", "ENV", "PATH", "REPO", "FILE",
|
||||
"I", "A", "AN", "THE", "THIS", "THAT", "IT", "ITS",
|
||||
"AND", "OR", "NOT", "BUT", "FOR", "OF", "ON", "TO", "IN", "IS",
|
||||
})
|
||||
|
||||
|
||||
def _cross_check_findings(
|
||||
findings: List[Dict[str, Any]],
|
||||
repo_root: Optional[pathlib.Path] = None,
|
||||
) -> Tuple[List[Dict[str, Any]], Dict[str, Any]]:
|
||||
"""Verify factual claims in critical findings against actual repo content.
|
||||
|
||||
For each finding whose ``severity`` is ``"critical"``: extract code-shape
|
||||
identifiers from its ``reason`` and ``item`` text, then verify each
|
||||
identifier exists somewhere in ``repo_root``. If EVERY mentioned
|
||||
identifier is absent AND the reason carries a strong claim (imports,
|
||||
uses, missing, typo, should-be, …), downgrade ``severity`` to
|
||||
``"advisory"`` and append a note to ``reason`` so the caller's gate
|
||||
sees it but the audit trail stays whole.
|
||||
|
||||
The cross-check is conservative by design:
|
||||
- It only intervenes when concrete identifiers can be extracted.
|
||||
- It only downgrades when ALL mentioned identifiers are absent.
|
||||
- Vague or prose findings without verifiable identifiers are
|
||||
passed through unchanged.
|
||||
- It is OFF when ``repo_root`` is None — existing call sites see no
|
||||
behavioral change unless they opt in.
|
||||
|
||||
Returns ``(findings, audit)``. ``audit`` records every cross-check
|
||||
decision so a downgraded finding is auditable end-to-end. The audit
|
||||
dict shape is stable: ``checked`` (int), ``downgraded`` (int),
|
||||
``kept`` (int), ``entries`` (list of per-finding dicts).
|
||||
"""
|
||||
audit: Dict[str, Any] = {
|
||||
"checked": 0,
|
||||
"downgraded": 0,
|
||||
"kept": 0,
|
||||
"entries": [],
|
||||
}
|
||||
if repo_root is None or not findings:
|
||||
return list(findings), audit
|
||||
try:
|
||||
repo_root_resolved = pathlib.Path(repo_root).resolve()
|
||||
except (OSError, TypeError, ValueError):
|
||||
return list(findings), audit
|
||||
if not (repo_root_resolved.exists() and repo_root_resolved.is_dir()):
|
||||
return list(findings), audit
|
||||
|
||||
out: List[Dict[str, Any]] = []
|
||||
for finding in findings:
|
||||
if not isinstance(finding, dict):
|
||||
out.append(finding)
|
||||
continue
|
||||
if str(finding.get("severity", "")).lower() != "critical":
|
||||
out.append(finding)
|
||||
continue
|
||||
|
||||
audit["checked"] += 1
|
||||
reason = str(finding.get("reason", "") or "")
|
||||
item = str(finding.get("item", "") or "")
|
||||
haystack = f"{reason}\n{item}"
|
||||
|
||||
candidates: List[str] = []
|
||||
seen: set = set()
|
||||
# Process patterns in length-descending order so the longest
|
||||
# identifier wins; shorter sub-tokens (e.g., the ``Claudexor`` piece
|
||||
# of ``Claudexor.unavailable``) are then skipped as substrings of
|
||||
# something already accepted. This avoids two-name "comparative"
|
||||
# patterns (Compare ``A`` with ``B``) where the reviewer is anchoring
|
||||
# to one real identifier and mentioning another; we still treat that
|
||||
# as a separate claim and let the all-absent rule keep it critical.
|
||||
ordered_patterns = (
|
||||
list(_BACKTICK_RE.finditer(haystack))
|
||||
+ list(_DOTTED_PATH_RE.finditer(haystack))
|
||||
+ list(_CAMEL_CASE_RE.finditer(haystack))
|
||||
)
|
||||
matches_by_length = sorted(
|
||||
ordered_patterns, key=lambda m: len(m.group(1)), reverse=True,
|
||||
)
|
||||
for m in matches_by_length:
|
||||
cand = m.group(1).strip().rstrip(".,;:")
|
||||
if not cand or cand in _NON_CODE_TOKENS or cand in seen:
|
||||
continue
|
||||
# Skip if ``cand`` is a strict substring of an already-accepted
|
||||
# candidate (``Claudexor.unavailable`` already in; ``Claudexor``
|
||||
# alone is just a prefix of that and should not count as a
|
||||
# separate claim).
|
||||
if any(cand in existing and existing != cand for existing in seen):
|
||||
continue
|
||||
seen.add(cand)
|
||||
candidates.append(cand)
|
||||
if not candidates:
|
||||
audit["kept"] += 1
|
||||
out.append(finding)
|
||||
continue
|
||||
|
||||
absent = [
|
||||
c for c in candidates
|
||||
if not _identifier_present_in_repo(c, repo_root_resolved)
|
||||
]
|
||||
makes_strong_claim = bool(_STRONG_CLAIM_RE.search(haystack))
|
||||
|
||||
if absent and len(absent) == len(candidates) and makes_strong_claim:
|
||||
new_finding = dict(finding)
|
||||
new_finding["severity"] = "advisory"
|
||||
note = (
|
||||
f"[cross-check] Downgraded from critical to advisory by "
|
||||
f"review_cross_check._cross_check_findings: identifier(s) "
|
||||
f"{absent!r} not located in repo; the reviewer's factual "
|
||||
f"claim cannot be substantiated against the codebase. "
|
||||
f"Re-raise as critical only after the identifier is verified "
|
||||
f"against actual code."
|
||||
)
|
||||
base_reason = new_finding.get("reason", "") or ""
|
||||
new_finding["reason"] = (
|
||||
f"{base_reason}\n\n{note}" if base_reason else note
|
||||
)
|
||||
audit["downgraded"] += 1
|
||||
audit["entries"].append({
|
||||
"item": item,
|
||||
"absent_identifiers": absent,
|
||||
"all_candidates": candidates,
|
||||
"action": "downgraded_to_advisory",
|
||||
})
|
||||
out.append(new_finding)
|
||||
continue
|
||||
|
||||
audit["kept"] += 1
|
||||
out.append(finding)
|
||||
return out, audit
|
||||
|
||||
|
|
@ -20,10 +20,6 @@ from dataclasses import dataclass
|
|||
from enum import Enum
|
||||
from typing import TYPE_CHECKING, Any, Callable, ClassVar, Dict, List, Optional
|
||||
|
||||
from ouroboros.review_cross_check import ( # noqa: F401 — re-exported: tests import from here
|
||||
_cross_check_findings,
|
||||
_identifier_present_in_repo,
|
||||
)
|
||||
from ouroboros.review_slot_cancel import ( # noqa: F401 — re-exported seam surface
|
||||
ReviewSessionSucceededResultUnavailable,
|
||||
_cancel_honesty_clause,
|
||||
|
|
@ -748,7 +744,7 @@ def run_delegated_review_session(
|
|||
from ouroboros import delegate_custody as custody
|
||||
from ouroboros.claudexor_daemon import ensure_owned_gateway
|
||||
from ouroboros.gateways.claudexor import (
|
||||
WINDOW_EXHAUSTED_CODES, ClaudexorSubscriptionWindowExhausted, ClaudexorUnavailable,
|
||||
WINDOW_EXHAUSTED_CODES, ClaudexorSubscriptionWindowExhausted, ClaudexorUnavailable, final_attempt_facts,
|
||||
)
|
||||
from ouroboros.subagents import delegated_run_shape, route_health
|
||||
from ouroboros.usage_accounting import current_usage_scope
|
||||
|
|
@ -972,6 +968,7 @@ def run_delegated_review_session(
|
|||
raise
|
||||
settlement = custody.settle_run(custody_drive, gateway, entry, detail)
|
||||
summary = custody.summary_of(detail)
|
||||
observed = final_attempt_facts(detail, run_id)
|
||||
run_state = str(summary.get("state") or "")
|
||||
if run_state != "succeeded":
|
||||
failure = summary.get("failure") if isinstance(summary.get("failure"), dict) else {}
|
||||
|
|
@ -991,7 +988,7 @@ def run_delegated_review_session(
|
|||
from ouroboros.review_thread_continuity import review_thread_receipt as receipt_for
|
||||
thread_receipt = receipt_for(gateway, thread_id, run_id, turn_id,
|
||||
expected_profile=str(getattr(route, "profile_id", "") or ""),
|
||||
applied_profile=str((summary.get("authRoute") or {}).get("profileId") or ""))
|
||||
applied_profile=observed.get("profile_id", ""))
|
||||
turn_id = str(thread_receipt.get("turn_id") or turn_id)
|
||||
state.pop("pending_invocation_id", None)
|
||||
state.pop("delegated_run_id", None)
|
||||
|
|
@ -1008,15 +1005,14 @@ def run_delegated_review_session(
|
|||
"idempotent_recovery": recovering,
|
||||
"settlement": settlement,
|
||||
"route_id": str(entry.route_id),
|
||||
# Engine receipt of the used pool; pinned-route drift is disclosed (D4).
|
||||
"effective_route_ids": [
|
||||
str(h) for h in (summary.get("harnesses") or []) if str(h)
|
||||
],
|
||||
"model": str(summary.get("model") or ""),
|
||||
# One final attempt, never the requested pool or a mixed summary route.
|
||||
"effective_route_ids": [observed["harness_id"]] if observed.get("harness_id") else [],
|
||||
"observed_attempt": observed,
|
||||
"model": observed.get("model", ""),
|
||||
"spend": spend,
|
||||
"spend_estimated": estimated,
|
||||
# D22/D29 applied facts are verbatim telemetry, never inferred.
|
||||
"applied_profile": str((summary.get("authRoute") or {}).get("profileId") or ""),
|
||||
"applied_profile": observed.get("profile_id", ""),
|
||||
"auth_route_receipt": summary.get("authRoute") or {},
|
||||
# Only effectiveAccess witnesses applied access; request echo is insufficient.
|
||||
"applied_access": str(summary.get("effectiveAccess") or ""),
|
||||
|
|
@ -1216,7 +1212,7 @@ class AgentSessionReviewExecutor(ReviewSlotExecutor):
|
|||
started = bool(self._run_id or getattr(exc, "delegated_run_started", False))
|
||||
if started and not self._session_usage_observed and session_usage_once(self._run_id):
|
||||
self._observe_usage({
|
||||
"provider": "claudexor", "resolved_model": str(self.assignment.slot.model or ""),
|
||||
"provider": "claudexor", "resolved_model": "",
|
||||
"delegated_run_started": True, "delegated_run_id": self._run_id, "cost": None,
|
||||
})
|
||||
self._session_usage_observed = True
|
||||
|
|
@ -1364,7 +1360,9 @@ class AgentSessionReviewExecutor(ReviewSlotExecutor):
|
|||
"provider": "claudexor",
|
||||
"resolved_model": facts["model"],
|
||||
"delegated_run_id": facts["run_id"],
|
||||
"delegated_route": facts["route_id"],
|
||||
"delegated_route": effective_routes[0] if len(effective_routes) == 1 else "",
|
||||
"requested_route": facts["route_id"],
|
||||
"observed_attempt": facts.get("observed_attempt") or {},
|
||||
"review_thread_id": str(facts.get("thread_id") or ""),
|
||||
"review_turn_id": str(facts.get("turn_id") or ""),
|
||||
"review_thread_receipt": facts.get("thread_receipt") or {},
|
||||
|
|
@ -1418,21 +1416,11 @@ class AgentSessionReviewExecutor(ReviewSlotExecutor):
|
|||
|
||||
def _verdict_result(self, force_extraction: bool = False) -> ReviewAttemptResult:
|
||||
text = self._raw_transcript or ""
|
||||
# Cross-check: when the request carries a session_root, pass it so
|
||||
# critical findings whose factual claims cannot be substantiated against
|
||||
# the actual code are downgraded to advisory before the obligation gate
|
||||
# sees them. Empty / None disables it (every non-session caller).
|
||||
session_root = (
|
||||
str(getattr(self.assignment.request, "session_root", "") or "")
|
||||
if self.assignment is not None
|
||||
else ""
|
||||
)
|
||||
canonical, method, extraction_usage = canonicalize_session_verdict(
|
||||
text,
|
||||
conformance_passed=self._conformance_passed and not force_extraction,
|
||||
contract=self._output_contract(),
|
||||
llm=self.llm,
|
||||
repo_root=session_root or None,
|
||||
deadline_at=getattr(self.assignment.request, "deadline_at", "") or "",
|
||||
transport_timeout_sec=getattr(self.assignment.slot, "transport_timeout_sec", None),
|
||||
shape=review_output_shape(self.assignment.request.surface),
|
||||
|
|
@ -1457,12 +1445,6 @@ class AgentSessionReviewExecutor(ReviewSlotExecutor):
|
|||
),
|
||||
"reason": "extraction_incomplete_transcript_exceeds_bound",
|
||||
}]
|
||||
# ibl-01b310c0ce18: promote the cross_check audit to top-level usage so
|
||||
# it survives the downstream merge (schema/strict branches return it in
|
||||
# `extraction_usage`; the light branch keeps its own under `extraction`).
|
||||
cc_audit = extraction_usage.get("cross_check")
|
||||
if cc_audit and not usage.get("cross_check"):
|
||||
usage["cross_check"] = cc_audit
|
||||
usage["verdict_method"] = method
|
||||
# P1: the cognitive artifact is the SESSION's own output, and canonicalization
|
||||
# legitimately destroys it — a schema-conformant `{"findings": []}` becomes `[]`
|
||||
|
|
|
|||
|
|
@ -115,9 +115,9 @@ def review_executions_from_actor_usage(actors: Any) -> List[Dict[str, str]]:
|
|||
usage = actor.get("usage") if isinstance(actor.get("usage"), dict) else {}
|
||||
delegated_route = str(usage.get("delegated_route") or "").strip()
|
||||
model = str(usage.get("resolved_model") or "").strip()
|
||||
if delegated_route:
|
||||
if delegated_route or (usage.get("provider") == "claudexor" and usage.get("delegated_run_id")):
|
||||
executions.append({
|
||||
"kind": "harness", "harness_id": delegated_route,
|
||||
"kind": "harness", **({"harness_id": delegated_route} if delegated_route else {}),
|
||||
**({"model": model} if model else {}),
|
||||
})
|
||||
elif _has_api_execution_receipt(usage):
|
||||
|
|
|
|||
|
|
@ -11,12 +11,10 @@ from __future__ import annotations
|
|||
|
||||
import json
|
||||
import logging
|
||||
import pathlib
|
||||
from typing import Any, Dict, List, Optional
|
||||
from typing import Any, Callable, Dict, List, Optional
|
||||
|
||||
from ouroboros.config import get_finalization_grace_sec
|
||||
from ouroboros.deadline_utils import owner_deadline_exhausted, review_transport_timeout
|
||||
from ouroboros.review_cross_check import _cross_check_findings
|
||||
from ouroboros.triad_review import (
|
||||
default_output_contract,
|
||||
empty_array_is_verified_clean,
|
||||
|
|
@ -91,7 +89,9 @@ def _findings_array(payload: Any) -> Optional[List[Dict[str, Any]]]:
|
|||
return None
|
||||
|
||||
|
||||
def _strictly_parseable(text: str, shape: str = "array") -> bool:
|
||||
def _strictly_parseable(
|
||||
text: str, shape: str = "array", array_validator: Optional[Callable[[list], bool]] = None,
|
||||
) -> bool:
|
||||
"""Would the surfaces' own strict parsers accept this text as a verdict?
|
||||
|
||||
The strict path comes FIRST (D19): a session that already obeyed the output
|
||||
|
|
@ -127,25 +127,28 @@ def _strictly_parseable(text: str, shape: str = "array") -> bool:
|
|||
parsed = json.loads(body.strip())
|
||||
except (TypeError, ValueError):
|
||||
return False
|
||||
return bool(parsed) and isinstance(parsed, list) and all(isinstance(item, dict) for item in parsed)
|
||||
return bool(parsed) and isinstance(parsed, list) and all(isinstance(item, dict) for item in parsed) \
|
||||
and (array_validator is None or array_validator(parsed))
|
||||
|
||||
|
||||
def _canonical_payload_text(payload: Any, shape: str) -> Optional[str]:
|
||||
def _canonical_payload_text(
|
||||
payload: Any, shape: str, array_validator: Optional[Callable[[list], bool]] = None,
|
||||
) -> Optional[str]:
|
||||
"""Canonical text of a structured payload for ``shape``, or None when it
|
||||
does not carry the contract's shape."""
|
||||
if shape == "object":
|
||||
verdict = object_verdict_payload(payload)
|
||||
return None if verdict is None else json.dumps(verdict, ensure_ascii=False)
|
||||
findings = _findings_array(payload)
|
||||
if findings is None:
|
||||
if findings is None or (findings and array_validator is not None and not array_validator(findings)):
|
||||
return None
|
||||
return "[]" if not findings else json.dumps(findings, ensure_ascii=False)
|
||||
|
||||
|
||||
def canonicalize_session_verdict(
|
||||
raw_text: str, *, conformance_passed: bool, contract: str = "", llm: Any = None,
|
||||
repo_root: Optional[str] = None,
|
||||
deadline_at: Any = None, transport_timeout_sec: Any = None, shape: str = "array",
|
||||
array_validator: Optional[Callable[[list], bool]] = None,
|
||||
) -> tuple[str, str, Dict[str, Any]]:
|
||||
"""Return ``(canonical_text, method, extraction_usage)`` for a session answer.
|
||||
|
||||
|
|
@ -166,73 +169,32 @@ def canonicalize_session_verdict(
|
|||
(a free-form product that is passed through verbatim; nothing here may
|
||||
turn a diagnosis into a findings array).
|
||||
|
||||
``repo_root`` (optional, ibl-01b310c0ce18): when set, every parsed findings
|
||||
array is passed through :func:`_cross_check_findings` before re-serialization
|
||||
— critical findings whose factual claims cannot be substantiated against the
|
||||
codebase are downgraded to ``"advisory"`` with an audit note. OFF by default
|
||||
(``repo_root=None``) so existing callers see no behavioural change.
|
||||
An array surface may supply its existing row validator. Shape-valid but
|
||||
contract-invalid rows then reach the same extraction rail; the host never
|
||||
guesses a verdict or severity. Other surfaces keep their own parsing rules.
|
||||
"""
|
||||
text = str(raw_text or "")
|
||||
if shape == "report":
|
||||
return text, "report", {}
|
||||
cc_audit: Dict[str, Any] = {}
|
||||
if conformance_passed:
|
||||
try:
|
||||
payload = json.loads(text.strip())
|
||||
except (TypeError, ValueError):
|
||||
payload = None
|
||||
canonical = _canonical_payload_text(payload, shape)
|
||||
canonical = _canonical_payload_text(payload, shape, array_validator)
|
||||
if canonical is not None:
|
||||
if shape == "array" and repo_root:
|
||||
findings, cc_audit = _cross_check_findings(
|
||||
_findings_array(payload), pathlib.Path(repo_root),
|
||||
)
|
||||
canonical = "[]" if not findings else json.dumps(findings, ensure_ascii=False)
|
||||
usage = {"cross_check": cc_audit} if cc_audit.get("checked") else {}
|
||||
return canonical, "schema", usage
|
||||
return canonical, "schema", {}
|
||||
# The engine claimed conformance over a payload that does not carry the
|
||||
# contract's shape: fall through to the honest branches, and the caller
|
||||
# discloses the delta.
|
||||
if _strictly_parseable(text, shape):
|
||||
try:
|
||||
findings_strict = json.loads(text.strip())
|
||||
if (
|
||||
isinstance(findings_strict, list)
|
||||
and all(isinstance(it, dict) for it in findings_strict)
|
||||
and repo_root
|
||||
):
|
||||
findings_strict, cc_audit = _cross_check_findings(
|
||||
findings_strict, pathlib.Path(repo_root),
|
||||
)
|
||||
# Only re-serialize when a downgrade actually happened —
|
||||
# otherwise the reviewer's exact bytes (and the provenance
|
||||
# hash computed from them) are preserved unchanged.
|
||||
if cc_audit.get("downgraded"):
|
||||
text = json.dumps(findings_strict, ensure_ascii=False)
|
||||
except (TypeError, ValueError):
|
||||
pass
|
||||
usage = {"cross_check": cc_audit} if cc_audit.get("checked") else {}
|
||||
return text, "strict", usage
|
||||
if _strictly_parseable(text, shape, array_validator):
|
||||
return text, "strict", {}
|
||||
if len(text) > _EXTRACT_MAX_CHARS:
|
||||
return text, "extraction_incomplete", {}
|
||||
canonical, usage = _extract_verdict_via_light_model(
|
||||
text, contract=contract, llm=llm, deadline_at=deadline_at,
|
||||
transport_timeout_sec=transport_timeout_sec, shape=shape)
|
||||
transport_timeout_sec=transport_timeout_sec, shape=shape, array_validator=array_validator)
|
||||
if canonical is not None:
|
||||
try:
|
||||
payload = json.loads(canonical)
|
||||
if (
|
||||
isinstance(payload, list)
|
||||
and all(isinstance(it, dict) for it in payload)
|
||||
and repo_root
|
||||
):
|
||||
payload, cc_audit = _cross_check_findings(payload, pathlib.Path(repo_root))
|
||||
if cc_audit.get("downgraded"):
|
||||
canonical = json.dumps(payload, ensure_ascii=False)
|
||||
except (TypeError, ValueError):
|
||||
pass
|
||||
if cc_audit.get("checked"):
|
||||
usage["cross_check"] = cc_audit
|
||||
return canonical, "light_model_extraction", usage
|
||||
# `unparsed` is the honest end of THIS layer's knowledge. The coordinator's
|
||||
# own fenced scanner may still parse the text downstream; labeling that
|
||||
|
|
@ -245,6 +207,7 @@ def canonicalize_session_verdict(
|
|||
def _extract_verdict_via_light_model(
|
||||
raw_text: str, *, contract: str = "", llm: Any = None, deadline_at: Any = None,
|
||||
transport_timeout_sec: Any = None, shape: str = "array",
|
||||
array_validator: Optional[Callable[[list], bool]] = None,
|
||||
) -> tuple[Optional[str], Dict[str, Any]]:
|
||||
"""One bounded light-model call canonicalizing narrative to the contract."""
|
||||
from ouroboros.config import get_light_model
|
||||
|
|
@ -317,4 +280,4 @@ def _extract_verdict_via_light_model(
|
|||
findings = None
|
||||
if findings is None:
|
||||
return None, usage
|
||||
return ("[]" if not findings else json.dumps(findings, ensure_ascii=False)), usage
|
||||
return _canonical_payload_text(findings, shape, array_validator), usage
|
||||
|
|
|
|||
|
|
@ -291,10 +291,9 @@ def record_last_delegation(*, route: str, requested_model: str,
|
|||
Best-effort and atomic, in the CANONICAL data plane beside the saved
|
||||
settings (the reviewer-slot projection's own rule): this is UI state, not
|
||||
per-task forensics — those live in the custody event log and the ledger.
|
||||
``applied_model`` is the engine summary's own value, '' when the run never
|
||||
disclosed one — the requested model is never dressed up as the applied one.
|
||||
The same rule for the account (D-U5): ``applied_profile`` is the engine's
|
||||
``authRoute.profileId`` settlement receipt, '' when telemetry predates it;
|
||||
``applied_model`` and ``applied_profile`` come from the same final attempt
|
||||
in the engine's telemetry, '' when that attempt disclosed no such fact.
|
||||
Neither the requested model nor a prior attempt supplies missing evidence;
|
||||
``requested_profile`` is the pin the request carried ('' = rotation) — the
|
||||
two stay separate so a requested-vs-ran mismatch is disclosable, never
|
||||
rewritten.
|
||||
|
|
|
|||
|
|
@ -324,8 +324,6 @@ def _delegate_start(ctx: ToolContext, prompt: str, max_seconds: Optional[int] =
|
|||
target_root = ""
|
||||
authority_source = ""
|
||||
resource_ref: Dict[str, Any] = {}
|
||||
selected_subagent_id = ""
|
||||
config_fingerprint = ""
|
||||
retry_token = str(retry_of or "").strip()
|
||||
source_binding = prepare_work_order_start_binding(
|
||||
ctx, drive, retry_token, _canonical_work_order_fingerprint, text,
|
||||
|
|
@ -587,13 +585,14 @@ def _delegate_start(ctx: ToolContext, prompt: str, max_seconds: Optional[int] =
|
|||
durable=durable, recovering=recovering,
|
||||
invocation_id=invocation_id,
|
||||
snapshot_id=snapshot_id, target_root=target_root,
|
||||
baseline_sha=baseline_sha)
|
||||
baseline_sha=baseline_sha,
|
||||
engine_version=str(getattr(gateway, "engine_version", "") or ""))
|
||||
|
||||
|
||||
def _started_payload(handle: Dict[str, Any], run_id: str, route: Any, access: str,
|
||||
authority: "DelegatedRunShape", root: str, *, durable: bool,
|
||||
recovering: bool, invocation_id: str, snapshot_id: str, target_root: str,
|
||||
baseline_sha: str) -> str:
|
||||
baseline_sha: str, engine_version: str = "") -> str:
|
||||
"""The one author of delegate_start's started result (note + payload).
|
||||
|
||||
The AUTHORITY guidance and the CUSTODY warning are independent facts about the same
|
||||
|
|
@ -626,6 +625,7 @@ def _started_payload(handle: Dict[str, Any], run_id: str, route: Any, access: st
|
|||
"status": "started" if durable else "started_uncustodied",
|
||||
"run_id": run_id,
|
||||
"run_dir": handle.get("runDir"),
|
||||
"engine_version": engine_version,
|
||||
"route": route.route_id,
|
||||
"model": route.model,
|
||||
"effort": route.effort,
|
||||
|
|
@ -918,9 +918,9 @@ def _delegate_wait(ctx: ToolContext, run_id: str, wait_sec: Optional[int] = None
|
|||
route=entry.route_id, requested_model=entry.model,
|
||||
applied_model=str(payload.get("model") or ""), run_id=rid,
|
||||
selected_subagent_id=entry.selected_subagent_id,
|
||||
# Applied = the settlement receipt's authRoute fact (never invented); requested replays off STARTED.
|
||||
# Applied = the same final attempt as the model; requested replays off STARTED.
|
||||
requested_profile=entry.profile_id,
|
||||
applied_profile=str((summary.get("authRoute") or {}).get("profileId") or ""))
|
||||
applied_profile=str((payload.get("observed_attempt") or {}).get("profile_id") or ""))
|
||||
# D7 made load-bearing: settlement is where "paid for and never read"
|
||||
# becomes permanent, so the parent is told in WORDS here — not left to
|
||||
# infer it from `output_delivery.consumed`. Re-settling an already
|
||||
|
|
@ -1100,6 +1100,11 @@ def get_tools() -> List[ToolEntry]:
|
|||
"leaf before your first round (the startup receipt carries its run id): never start a duplicate — "
|
||||
"supervise it; a replacement delegate_start(prompt='') is legal only after verified cancellation/"
|
||||
"terminal settlement or a typed refusal proving no run exists. Recovery retries use retry_of without a new selector."
|
||||
" This ordinary call requests no extra Claudexor review panel; new ordinary "
|
||||
"runs on engine 3.9.8+ default to no panel. The started receipt names the serving "
|
||||
"engine_version; an older engine or a recovered historical run may retain its "
|
||||
"earlier review behavior. Engine review, execution success, your integration "
|
||||
"decision, and applicable Ouroboros review gates remain separate."
|
||||
),
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
|
|
|
|||
|
|
@ -173,15 +173,18 @@ def _containment_evidence(detail: Dict[str, Any]) -> Dict[str, Any]:
|
|||
|
||||
def _terminal_payload(run_id: str, detail: Dict[str, Any],
|
||||
authority: "DelegatedRunShape") -> Dict[str, Any]:
|
||||
from ouroboros.gateways.claudexor import final_attempt_facts
|
||||
|
||||
summary = _delegate().custody.summary_of(detail)
|
||||
observed = final_attempt_facts(detail, run_id)
|
||||
payload = {
|
||||
"status": "terminal",
|
||||
"run_id": run_id,
|
||||
"state": str(summary.get("state") or ""),
|
||||
# The APPLIED model, from the engine's own summary — '' when the run
|
||||
# never disclosed one (live unpinned runs really do), shown as absence
|
||||
# rather than the requested model dressed up as the applied one.
|
||||
"model": str(summary.get("model") or ""),
|
||||
# Model, harness and profile come from the SAME final attempt. The
|
||||
# summary's requested model and cross-attempt route are not evidence.
|
||||
"model": observed.get("model", ""),
|
||||
"observed_attempt": observed,
|
||||
"outcome_banner": detail.get("outcomeBanner"),
|
||||
"outcome_facts": summary.get("outcomeFacts"),
|
||||
"output_conformance": summary.get("outputConformance"),
|
||||
|
|
|
|||
|
|
@ -61,9 +61,9 @@ _ADVISORY_EXTRACT_CONTRACT = (
|
|||
'"item" (checklist item name), "verdict" ("PASS" or "FAIL"), "severity" '
|
||||
'("critical" or "advisory" — REQUIRED even for PASS entries), "reason" (brief '
|
||||
'explanation). Optional: "obligation_id" (stable id of a previously surfaced '
|
||||
"obligation). If a FAIL entry in the source omits severity, infer it from "
|
||||
'context: "critical" for bugs, security or constitutional violations, else '
|
||||
'"advisory". If the text carries no valid checklist array, return [].'
|
||||
"obligation). Preserve the reviewer's stated verdict and severity; do not "
|
||||
"infer severity from the alleged bug or change a verdict from withdrawal prose. "
|
||||
"If either cannot be faithfully recovered, return UNEXTRACTABLE."
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -96,6 +96,7 @@ def _llm_extract_advisory_items(raw_text: str, ctx: object) -> list:
|
|||
# the trusted-schema branch is never taken on this path.
|
||||
conformance_passed=False,
|
||||
contract=_ADVISORY_EXTRACT_CONTRACT,
|
||||
array_validator=_is_checklist_array,
|
||||
deadline_at=(getattr(ctx, "task_metadata", {}) or {}).get("deadline_at"),
|
||||
)
|
||||
if method == "extraction_incomplete":
|
||||
|
|
@ -120,23 +121,9 @@ def _llm_extract_advisory_items(raw_text: str, ctx: object) -> list:
|
|||
provider=_infer_prov(light_model),
|
||||
)
|
||||
|
||||
# The SSOT already flattened provider content blocks to text; the advisory's
|
||||
# OWN contract post-processing (below) is unchanged and stays here.
|
||||
items = _parse_advisory_output(str(content or ""))
|
||||
if not _is_checklist_array(items):
|
||||
return []
|
||||
|
||||
# Missing FAIL severity defaults to critical; never silently downgrade.
|
||||
normalised = []
|
||||
for it in items:
|
||||
if not isinstance(it, dict):
|
||||
continue
|
||||
verdict = str(it.get("verdict", "")).upper().strip()
|
||||
if verdict == "FAIL" and not str(it.get("severity", "")).strip():
|
||||
it = dict(it)
|
||||
it["severity"] = "critical"
|
||||
normalised.append(it)
|
||||
return normalised
|
||||
# Canonical enum spelling is shared with direct parsing. Missing or
|
||||
# unknown severity stays unparsed, never a host-authored critical.
|
||||
return _parse_advisory_output(str(content or ""))
|
||||
|
||||
except Exception as exc:
|
||||
log.warning("Advisory LLM fallback extraction failed: %s", exc)
|
||||
|
|
@ -345,7 +332,7 @@ def _run_advisory_delegated(prompt: str, repo_dir: pathlib.Path, ctx: ToolContex
|
|||
usage={}, error=f"{type(exc).__name__}: {exc}", stderr_tail="",
|
||||
), ""
|
||||
usage = dict(attempt.usage or {})
|
||||
resolved_model = str(usage.get("resolved_model") or usage.get("delegated_route") or "")
|
||||
resolved_model = str(usage.get("resolved_model") or "")
|
||||
return SimpleNamespace(
|
||||
success=True,
|
||||
result_text=str(attempt.raw_text or ""),
|
||||
|
|
@ -657,23 +644,34 @@ def _needs_fallback_extraction(items: list, raw_text: str) -> bool:
|
|||
|
||||
def _parse_advisory_output(stdout: str) -> list:
|
||||
"""Extract the JSON findings array from Claude CLI output."""
|
||||
return _car().extract_json_array(
|
||||
items = _car().extract_json_array(
|
||||
stdout,
|
||||
unwrap_result=True,
|
||||
validate_fn=_is_checklist_array,
|
||||
) or []
|
||||
return [dict(item, verdict=item["verdict"].strip().upper(), **(
|
||||
{"severity": item["severity"].strip().lower()} if "severity" in item else {}
|
||||
)) for item in items]
|
||||
|
||||
|
||||
def _is_checklist_array(items: list) -> bool:
|
||||
"""Return True iff items looks like a real advisory checklist array.
|
||||
|
||||
Each element must be a dict containing at least 'item' and 'verdict' keys.
|
||||
An empty list is rejected (no findings = parse_failure, not a clean advisory).
|
||||
Stray arrays like [1,2,3], code snippets, or unrelated JSON lists are rejected.
|
||||
Unknown enum values invalidate the whole array, never silently dropping a
|
||||
finding or choosing its seriousness. PASS without severity stays compatible;
|
||||
FAIL requires the reviewer's critical/advisory classification. Empty clean
|
||||
responses are recognized separately by _is_clean_verdict.
|
||||
"""
|
||||
if not items:
|
||||
if not isinstance(items, list) or not items:
|
||||
return False
|
||||
return all(
|
||||
isinstance(el, dict) and "item" in el and "verdict" in el
|
||||
isinstance(el, dict)
|
||||
and isinstance(el.get("item"), str) and bool(el["item"].strip())
|
||||
and isinstance(el.get("verdict"), str) and el["verdict"].strip().upper() in {"PASS", "FAIL"}
|
||||
and (
|
||||
(el["verdict"].strip().upper() == "PASS" and "severity" not in el)
|
||||
or isinstance(el.get("severity"), str)
|
||||
and el["severity"].strip().lower() in {"critical", "advisory"}
|
||||
)
|
||||
for el in items
|
||||
)
|
||||
|
|
|
|||
|
|
@ -975,7 +975,11 @@ def record_subscription_session(
|
|||
access_profile: str = "",
|
||||
review_skill: str = "", review_wave_id: str = "", review_slot_id: str = "",
|
||||
) -> str:
|
||||
"""Record one idempotent subscription session; None remains undisclosed."""
|
||||
"""Record one idempotent session; model observation is not session identity.
|
||||
|
||||
A later model disclosure replays the existing row byte-for-byte, without
|
||||
repricing or rewriting it. Custody carries the newly observed actor facts.
|
||||
"""
|
||||
stable_id, route_id = str(session_id or "").strip(), str(route or "").strip()
|
||||
if not stable_id or not route_id:
|
||||
raise UsageAccountingError("subscription session requires a stable session_id and route")
|
||||
|
|
@ -1018,7 +1022,7 @@ def record_subscription_session(
|
|||
"model_send_seal": "unobserved",
|
||||
}
|
||||
return _append_single_settled_row(root, row, comparable=(
|
||||
"kind", "model", "provider", "task_id", "root_task_id", "parent_task_id",
|
||||
"kind", "provider", "task_id", "root_task_id", "parent_task_id",
|
||||
"category", "source", *REVIEW_ATTRIBUTION_KEYS, "subscription_route", "session_id_sha256",
|
||||
))
|
||||
def _transition(reservation: AttemptReservation, state: str, **fields: Any) -> Dict[str, Any]:
|
||||
|
|
|
|||
|
|
@ -69,6 +69,8 @@ class FakeGateway:
|
|||
artifact_error = None
|
||||
nonterminal = False
|
||||
project_unregistered = False
|
||||
telemetry = None
|
||||
run_dir = None
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
FakeGateway.instances.append(self)
|
||||
|
|
@ -103,6 +105,8 @@ class FakeGateway:
|
|||
cls.artifact_error = None
|
||||
cls.nonterminal = False
|
||||
cls.project_unregistered = False
|
||||
cls.telemetry = None
|
||||
cls.run_dir = None
|
||||
|
||||
def handshake(self, **_kw):
|
||||
return {"compatible": True, "protocolMajor": 3, "engine": {"version": self.engine_version}}
|
||||
|
|
@ -149,7 +153,20 @@ class FakeGateway:
|
|||
raise exc
|
||||
if FakeGateway.nonterminal:
|
||||
return {"summary": {"state": "running"}, "lastSeq": 1}
|
||||
return json.loads(json.dumps(FakeGateway.detail))
|
||||
detail = json.loads(json.dumps(FakeGateway.detail))
|
||||
if self.run_dir is not None:
|
||||
telemetry = FakeGateway.telemetry
|
||||
if telemetry is None:
|
||||
telemetry = {"run_id": run_id, "final_attempt_id": "a01", "attempts": [{
|
||||
"attempt_id": "a01", "harness_id": "fake-review",
|
||||
"observed_model": detail.get("summary", {}).get("model"), "profile_id": None,
|
||||
}]}
|
||||
final = self.run_dir / "final"
|
||||
final.mkdir(parents=True, exist_ok=True)
|
||||
# JSON is a YAML subset: same separate artifact as the real engine.
|
||||
(final / "telemetry.yaml").write_text(json.dumps(telemetry), encoding="utf-8")
|
||||
detail.setdefault("summary", {})["runDir"] = str(self.run_dir)
|
||||
return detail
|
||||
|
||||
def get_run_artifact(self, run_id, path):
|
||||
self.artifact_gets.append((run_id, path))
|
||||
|
|
@ -176,8 +193,9 @@ class FakeLLM:
|
|||
return {"content": self.reply}, {"prompt_tokens": 5, "completion_tokens": 2, "cost": 0.0001}
|
||||
|
||||
@pytest.fixture()
|
||||
def fake_route(monkeypatch):
|
||||
def fake_route(monkeypatch, tmp_path):
|
||||
FakeGateway.reset()
|
||||
FakeGateway.run_dir = tmp_path / "review-run"
|
||||
monkeypatch.setattr("ouroboros.gateways.claudexor.ClaudexorGateway", FakeGateway)
|
||||
monkeypatch.setenv(REVIEW_SESSION_ROUTE_ENV, "fake-review=fake-small:low")
|
||||
# ABI-10: the phase-5 per-row route envs are retired and IGNORED; nothing
|
||||
|
|
|
|||
|
|
@ -686,7 +686,7 @@ class TestIsChecklistArray:
|
|||
def test_valid_multi_item_accepted(self):
|
||||
items = [
|
||||
{"item": "bible_compliance", "verdict": "PASS"},
|
||||
{"item": "code_quality", "verdict": "FAIL", "reason": "bug"},
|
||||
{"item": "code_quality", "verdict": "FAIL", "severity": "critical", "reason": "bug"},
|
||||
]
|
||||
assert self.fn(items) is True
|
||||
|
||||
|
|
@ -1029,9 +1029,8 @@ class TestLLMFallbackExtraction:
|
|||
model = self.mod._resolve_fallback_model()
|
||||
assert model == "openai::gpt-4o-mini"
|
||||
|
||||
def test_fallback_normalises_fail_without_severity_to_critical(self, monkeypatch):
|
||||
"""FAIL items missing 'severity' from the LLM fallback must be normalised to 'critical'
|
||||
so _handle_advisory_pre_review() never silently downgrades blocking findings."""
|
||||
def test_fallback_does_not_invent_severity_for_an_incomplete_finding(self, monkeypatch):
|
||||
"""An extractor's incomplete FAIL remains unparsed, not host-classified."""
|
||||
# Simulate LLM returning a FAIL item with no severity (schema-incomplete output)
|
||||
raw_items = [
|
||||
{"item": "code_quality", "verdict": "FAIL", "reason": "bug found"},
|
||||
|
|
@ -1044,11 +1043,7 @@ class TestLLMFallbackExtraction:
|
|||
monkeypatch.setattr(llm_mod.LLMClient, "chat", fake_chat)
|
||||
|
||||
result = self.mod._llm_extract_advisory_items("narrative with no json", self._make_ctx())
|
||||
assert len(result) == 1
|
||||
assert result[0]["verdict"] == "FAIL"
|
||||
assert result[0]["severity"] == "critical", (
|
||||
"FAIL without severity must be normalised to 'critical' — not left empty"
|
||||
)
|
||||
assert result == []
|
||||
|
||||
def test_fallback_returns_empty_on_llm_failure(self, monkeypatch):
|
||||
"""When the LLM call raises an exception, fallback must return [] gracefully."""
|
||||
|
|
@ -1123,7 +1118,7 @@ class TestAdvisoryCleanSentinel:
|
|||
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-fake-for-test")
|
||||
monkeypatch.setattr(
|
||||
adv, "_run_claude_advisory",
|
||||
lambda repo_dir, commit_message, ctx, **kwargs: ([], raw_text, "opus", 10),
|
||||
lambda repo_dir, commit_message, ctx, **kwargs: (adv._parse_advisory_output(raw_text), raw_text, "opus", 10),
|
||||
)
|
||||
# Release-metadata preflight (BIBLE P9) runs before the SDK branch under
|
||||
# test, so the fake change set must carry the release artifacts.
|
||||
|
|
@ -1177,6 +1172,21 @@ class TestAdvisoryCleanSentinel:
|
|||
result = self._run_handler(tmp_path, monkeypatch, '[{"item": broken\nNO_FINDINGS')
|
||||
assert result.get("status") == "parse_failure"
|
||||
|
||||
@pytest.mark.parametrize("severity,expected", [("high", "parse_failure"), (" CRITICAL ", "fresh")])
|
||||
def test_row_contract_is_preserved_in_the_durable_advisory_result(self, tmp_path, monkeypatch, severity, expected):
|
||||
from ouroboros.tools import claude_advisory_review as advisory
|
||||
|
||||
raw = json.dumps([{"item": "check", "verdict": " fail ", "severity": severity, "reason": "bug"}])
|
||||
result = self._run_handler(tmp_path, monkeypatch, raw)
|
||||
assert result["status"] == expected
|
||||
record = advisory.load_state(tmp_path).latest()
|
||||
assert record.status == expected and record.raw_result == raw
|
||||
if expected == "fresh":
|
||||
assert result["critical_count"] == 1
|
||||
assert record.items[0]["severity"] == "critical"
|
||||
else:
|
||||
assert record.items == []
|
||||
|
||||
|
||||
class TestEmptyArrayIsVerifiedClean:
|
||||
"""The shared predicate both advisory and triad classify with. A reviewer
|
||||
|
|
|
|||
86
tests/test_advisory_row_contract.py
Normal file
86
tests/test_advisory_row_contract.py
Normal file
|
|
@ -0,0 +1,86 @@
|
|||
"""Advisory row validation preserves findings and honest unknown outcomes."""
|
||||
|
||||
import json
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from ouroboros.tools import preflight_review_run as run
|
||||
|
||||
|
||||
@pytest.mark.parametrize("row", [
|
||||
{"item": "check", "verdict": "OK", "severity": "critical"},
|
||||
{"item": "check", "verdict": "FAILED", "severity": "critical"},
|
||||
{"item": "check", "verdict": "FAIL", "severity": "high"},
|
||||
{"item": "check", "verdict": "FAIL"},
|
||||
{"item": "check", "verdict": "FAIL", "severity": None},
|
||||
{"item": "check", "verdict": True, "severity": "critical"},
|
||||
{"item": "check", "verdict": "PASS", "severity": "warning"},
|
||||
{"item": None, "verdict": "PASS"},
|
||||
])
|
||||
def test_invalid_row_rejects_whole_array_without_dropping_findings(row):
|
||||
raw = json.dumps([{"item": "valid", "verdict": "PASS"}, row])
|
||||
assert run._parse_advisory_output(raw) == []
|
||||
assert run._needs_fallback_extraction([], raw)
|
||||
assert not run._is_clean_verdict(raw)
|
||||
|
||||
|
||||
def test_enum_case_and_whitespace_are_normalized_once_without_changing_prose():
|
||||
rows = [{"item": "check", "verdict": " fail ", "severity": " CRITICAL ",
|
||||
"reason": "I withdraw this finding", "obligation_id": "ob-1"},
|
||||
{"item": "other", "verdict": " pass "}]
|
||||
parsed = run._parse_advisory_output(json.dumps(rows))
|
||||
assert parsed == [{**rows[0], "verdict": "FAIL", "severity": "critical"},
|
||||
{**rows[1], "verdict": "PASS"}]
|
||||
|
||||
|
||||
def test_malformed_json_array_reaches_existing_extraction_with_the_whole_source(monkeypatch):
|
||||
import ouroboros.llm as llm
|
||||
|
||||
raw = json.dumps([{"item": "check", "verdict": "FAILED", "severity": "critical",
|
||||
"reason": "The reviewer found a bug."}])
|
||||
extracted = [{"item": "check", "verdict": "FAIL", "severity": "critical",
|
||||
"reason": "The reviewer found a bug."}]
|
||||
calls = []
|
||||
|
||||
def chat(_self, **kwargs):
|
||||
calls.append(kwargs)
|
||||
return {"content": json.dumps(extracted)}, {"cost": 0.001}
|
||||
|
||||
monkeypatch.setattr(llm.LLMClient, "chat", chat)
|
||||
assert run._llm_extract_advisory_items(raw, SimpleNamespace()) == extracted
|
||||
assert len(calls) == 1
|
||||
assert raw in calls[0]["messages"][0]["content"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("extraction", ["UNEXTRACTABLE", "[]", "unchanged"])
|
||||
def test_unresolved_severity_never_becomes_clean_or_critical(monkeypatch, extraction):
|
||||
import ouroboros.llm as llm
|
||||
|
||||
raw = '[{"item":"check","verdict":"FAIL","severity":"high"}]'
|
||||
output = raw if extraction == "unchanged" else extraction
|
||||
monkeypatch.setattr(llm.LLMClient, "chat", lambda *a, **k: ({"content": output}, {}))
|
||||
assert run._llm_extract_advisory_items(raw, SimpleNamespace()) == []
|
||||
assert not run._is_clean_verdict(raw)
|
||||
|
||||
|
||||
def test_transport_to_advisory_consumer_preserves_normalized_findings(tmp_path, monkeypatch):
|
||||
from ouroboros.tools import claude_advisory_review as advisory
|
||||
from ouroboros import reviewer_slot_config
|
||||
|
||||
raw = '[{"item":"check","verdict":" fail ","severity":" CRITICAL ","reason":"bug"}]'
|
||||
monkeypatch.setattr(run, "advisory_review_route", lambda: "api_chat")
|
||||
monkeypatch.setattr(reviewer_slot_config, "advisory_slot_config", lambda: SimpleNamespace(effort="low"))
|
||||
monkeypatch.setattr("ouroboros.provider_models.model_has_credentials", lambda model: True)
|
||||
monkeypatch.setattr(advisory, "_advisory_native_model", lambda: "test/model")
|
||||
monkeypatch.setattr(advisory, "_build_advisory_prompt", lambda *a, **k: "review prompt")
|
||||
monkeypatch.setattr(advisory, "_predispatch_size_skip", lambda *a: None)
|
||||
monkeypatch.setattr(advisory, "_run_advisory_native", lambda *a, **k: (
|
||||
SimpleNamespace(success=True, result_text=raw, session_id="test", cost_usd=0,
|
||||
usage={}, error="", stderr_tail=""), "test/model"))
|
||||
ctx = SimpleNamespace(task_id="row-contract", task_metadata={})
|
||||
items, source, model, _ = run._run_claude_advisory(
|
||||
tmp_path, "test", ctx, options={"include_repo_diff": False},
|
||||
)
|
||||
assert items == [{"item": "check", "verdict": "FAIL", "severity": "critical", "reason": "bug"}]
|
||||
assert source == raw and model == "test/model"
|
||||
110
tests/test_claudexor_observed_attempt.py
Normal file
110
tests/test_claudexor_observed_attempt.py
Normal file
|
|
@ -0,0 +1,110 @@
|
|||
"""Final-attempt route facts follow Claudexor's telemetry artifact, not request echoes."""
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
from ouroboros.gateways.claudexor import final_attempt_facts
|
||||
|
||||
|
||||
def _write_telemetry(tmp_path, attempts, *, final_id="a02", run_id="run-fixture"):
|
||||
# Claudexor 3.9.8 RunTelemetry shape, reduced to the fields this reader owns.
|
||||
# Route values below were observed on all three harnesses; IDs are fixtures.
|
||||
path = tmp_path / "final" / "telemetry.yaml"
|
||||
path.parent.mkdir(parents=True, exist_ok=True)
|
||||
path.write_text(yaml.safe_dump({
|
||||
"schema_version": 2, "run_id": run_id, "task_id": "task-fixture",
|
||||
"final_attempt_id": final_id, "attempts": attempts,
|
||||
}), encoding="utf-8")
|
||||
return {"summary": {
|
||||
"runDir": str(tmp_path), "model": "request-echo", "harnesses": ["requested-harness"],
|
||||
"route": {"observedModel": "earlier-model", "harnessId": "earlier-harness", "verified": True},
|
||||
"authRoute": {"attemptId": "a01", "profileId": "earlier-profile"},
|
||||
}}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("harness,requested,observed", [
|
||||
("codex", "gpt-6-astra", "gpt-6-astra"),
|
||||
("claude", "claude-fable-5-1", "claude-fable-5-1"),
|
||||
("cursor", "cursor-grok-4.6-xhigh", "Cursor Grok 4.6 Extra High"),
|
||||
])
|
||||
def test_final_attempt_keeps_observed_values_together(tmp_path, harness, requested, observed):
|
||||
detail = _write_telemetry(tmp_path, [
|
||||
{"attempt_id": "a01", "harness_id": "earlier-harness", "observed_model": "earlier-model",
|
||||
"profile_id": "earlier-profile"},
|
||||
{"attempt_id": "a02", "harness_id": harness, "observed_model": observed,
|
||||
"requested_model": requested, "profile_id": "final-profile", "auth_mode": "local_session"},
|
||||
{"attempt_id": "a03", "harness_id": "later-harness", "observed_model": "later-model",
|
||||
"profile_id": "later-profile"},
|
||||
])
|
||||
|
||||
assert final_attempt_facts(detail, "run-fixture") == {
|
||||
"attempt_id": "a02", "harness_id": harness, "model": observed, "profile_id": "final-profile",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("missing", [None, "", 12, False, ["model"], {"model": "value"}])
|
||||
def test_final_attempt_missing_facts_never_borrow_from_earlier_attempt(tmp_path, missing):
|
||||
detail = _write_telemetry(tmp_path, [
|
||||
{"attempt_id": "a01", "harness_id": "earlier-harness", "observed_model": "earlier-model",
|
||||
"profile_id": "earlier-profile"},
|
||||
{"attempt_id": "a02", "harness_id": missing, "observed_model": missing,
|
||||
"requested_model": "request-echo", "profile_id": missing},
|
||||
])
|
||||
|
||||
assert final_attempt_facts(detail, "run-fixture") == {
|
||||
"attempt_id": "a02", "harness_id": "", "model": "", "profile_id": "",
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("attempts,final_id", [
|
||||
([{"attempt_id": "a01", "observed_model": "earlier-model"}], "a02"),
|
||||
([{"attempt_id": "a02"}, {"attempt_id": "a02"}], "a02"),
|
||||
([{"attempt_id": "a01", "observed_model": "earlier-model"}], None),
|
||||
([{"attempt_id": "a01", "observed_model": "earlier-model"}], ""),
|
||||
([{"attempt_id": "a01", "observed_model": "earlier-model"}], " "),
|
||||
([{"attempt_id": 2, "observed_model": "earlier-model"}], 2),
|
||||
({"a02": {"observed_model": "earlier-model"}}, "a02"),
|
||||
([None, "a02"], "a02"),
|
||||
])
|
||||
def test_unbound_or_ambiguous_final_attempt_is_unknown(tmp_path, attempts, final_id):
|
||||
detail = _write_telemetry(tmp_path, attempts, final_id=final_id)
|
||||
|
||||
assert final_attempt_facts(detail, "run-fixture") == {}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("run_id", ["other-run", "", None, 1])
|
||||
def test_telemetry_must_belong_to_the_requested_run(tmp_path, run_id):
|
||||
detail = _write_telemetry(tmp_path, [{"attempt_id": "a02", "observed_model": "actual"}])
|
||||
|
||||
assert final_attempt_facts(detail, run_id) == {}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("raw", [b"", b"null", b"[]", b"bad: [", b"\xff"])
|
||||
def test_unreadable_telemetry_stays_unknown(tmp_path, raw):
|
||||
detail = _write_telemetry(tmp_path, [{"attempt_id": "a02", "observed_model": "actual"}])
|
||||
(tmp_path / "final" / "telemetry.yaml").write_bytes(raw)
|
||||
|
||||
assert final_attempt_facts(detail, "run-fixture") == {}
|
||||
|
||||
|
||||
def test_missing_or_inaccessible_telemetry_stays_unknown(tmp_path, monkeypatch):
|
||||
detail = {"summary": {"runDir": str(tmp_path), "model": "request-echo"}}
|
||||
assert final_attempt_facts(detail, "run-fixture") == {}
|
||||
|
||||
def inaccessible(*args, **kwargs):
|
||||
raise PermissionError("fixture denies the artifact read")
|
||||
|
||||
monkeypatch.setattr(Path, "read_text", inaccessible)
|
||||
assert final_attempt_facts(detail, "run-fixture") == {}
|
||||
|
||||
|
||||
@pytest.mark.parametrize("detail", [None, [], {}, {"summary": []}, {"summary": {}},
|
||||
{"summary": {"runDir": ""}}, {"summary": {"runDir": 42}}])
|
||||
def test_missing_engine_run_directory_never_reads_the_working_directory(detail, monkeypatch):
|
||||
def unexpected_read(*args, **kwargs):
|
||||
raise AssertionError("missing runDir must not become a relative path")
|
||||
|
||||
monkeypatch.setattr(Path, "read_text", unexpected_read)
|
||||
assert final_attempt_facts(detail, "run-fixture") == {}
|
||||
|
|
@ -147,3 +147,37 @@ def test_verbose_text_remains_disclosed_and_the_payload_fits():
|
|||
|
||||
def test_no_new_rows_keeps_the_existing_progress_fallback():
|
||||
assert render() == "🛰 delegated run run-1 @seq 18: (new session events)"
|
||||
|
||||
|
||||
@pytest.mark.parametrize("event_type", ["reviewer.started", "reviewer.failed", "reviewer.timed_out"])
|
||||
def test_reviewer_events_preserve_the_known_actor_in_payload_and_live_progress(event_type):
|
||||
event = {"type": event_type, "title": "Reviewer setup failed", "severity": "error",
|
||||
"harnessId": "review-harness", "attemptId": "a02"}
|
||||
assert timeline_tail({"timeline": [event]}) == [event]
|
||||
seen = WindowObservations()
|
||||
advance = seen.record({"timeline": [event]}, 9, 3)
|
||||
output = []
|
||||
emit(SimpleNamespace(emit_progress_fn=output.append), "run-1", advance)
|
||||
assert output == ["🛰 delegated run run-1 @seq 9: [review-harness/a02] Reviewer setup failed"]
|
||||
assert seen.rows(10000)[0]["events"] == [event]
|
||||
|
||||
|
||||
def test_start_receipt_names_the_serving_engine_without_claiming_review_success():
|
||||
from ouroboros.subagents import delegated_run_shape
|
||||
from ouroboros.tools.delegate import _started_payload, get_tools
|
||||
from ouroboros.delegate_start_instructions import HOST_INSTRUCTIONS
|
||||
|
||||
payload = json.loads(_started_payload(
|
||||
{"runDir": "/fixture/run"}, "run-1", SimpleNamespace(route_id="selected", model="m", effort="high"),
|
||||
"readonly", delegated_run_shape(False), "/fixture/repo", durable=True, recovering=False,
|
||||
invocation_id="invocation-1", snapshot_id="", target_root="", baseline_sha="",
|
||||
engine_version="3.9.7",
|
||||
))
|
||||
assert payload["engine_version"] == "3.9.7"
|
||||
assert payload["status"] == "started"
|
||||
assert "review_passed" not in payload
|
||||
description = next(tool.schema["description"] for tool in get_tools() if tool.name == "delegate_start")
|
||||
assert "requests no extra Claudexor review panel" in description
|
||||
assert "recovered historical run" in description
|
||||
assert "self_worktree capture separately requires an unchanged HEAD" in HOST_INSTRUCTIONS
|
||||
assert "preserve committed changes" in HOST_INSTRUCTIONS
|
||||
|
|
|
|||
|
|
@ -187,6 +187,31 @@ def test_d29_absent_authroute_records_empty_never_invented(tmp_path, monkeypatch
|
|||
assert event["credential_profile_id"] == ""
|
||||
|
||||
|
||||
@pytest.mark.parametrize("observed", [
|
||||
{"harness_id": "actual-route", "observed_model": "actual-model", "profile_id": "actual-profile"},
|
||||
{},
|
||||
])
|
||||
def test_final_attempt_identity_survives_settlement_and_parent_delivery(tmp_path, monkeypatch, observed):
|
||||
from ouroboros.subagents import subagent_last_delegation
|
||||
|
||||
monkeypatch.setattr("ouroboros.config.DATA_DIR", tmp_path / "canonical-data")
|
||||
payload, ledger, event = _settled_run(tmp_path, monkeypatch, {
|
||||
"state": "succeeded", "spendUsd": 0.0, "model": "request-echo",
|
||||
"authRoute": {"profileId": "old-profile"}, "effectiveAccess": "readonly",
|
||||
}, observed=observed)
|
||||
for row in [payload, ledger, event]:
|
||||
assert row["model"] == observed.get("observed_model", "")
|
||||
assert ledger["credential_profile_id"] == observed.get("profile_id", "")
|
||||
assert event["credential_profile_id"] == observed.get("profile_id", "")
|
||||
assert event["observed_attempt"] == payload["observed_attempt"]
|
||||
assert payload["observed_attempt"]["attempt_id"] == "a02"
|
||||
assert ledger["cost_usd"] == 0.0 and ledger["cost_final"] is True
|
||||
record = subagent_last_delegation()
|
||||
assert record["requested_model"] == "m"
|
||||
assert record["applied_model"] == observed.get("observed_model", "")
|
||||
assert record["applied_profile"] == observed.get("profile_id", "")
|
||||
|
||||
|
||||
def test_the_durable_access_profile_is_the_receipt_never_our_own_request(tmp_path, monkeypatch):
|
||||
"""The daemon computes `access` as `effectiveAccess ?? the client's own parsed
|
||||
request`, so it is our ask reflected back, not a witness. Reading it as a fallback
|
||||
|
|
@ -197,7 +222,27 @@ def test_the_durable_access_profile_is_the_receipt_never_our_own_request(tmp_pat
|
|||
assert event["access_profile"] == ""
|
||||
|
||||
|
||||
def _settled_run(tmp_path, monkeypatch, summary):
|
||||
def _observed_summary(tmp_path, summary, observed=None):
|
||||
"""Give existing positive fixtures a separate engine telemetry artifact.
|
||||
|
||||
The summary values used by the old tests describe the intended observation;
|
||||
new mismatch/unknown scenarios pass an independent observation explicitly.
|
||||
"""
|
||||
final = tmp_path / "engine-run" / "final"
|
||||
final.mkdir(parents=True, exist_ok=True)
|
||||
if observed is None:
|
||||
observed = {"harness_id": "r", "observed_model": summary.get("model"),
|
||||
"profile_id": (summary.get("authRoute") or {}).get("profileId")}
|
||||
(final / "telemetry.yaml").write_text(json.dumps({
|
||||
"run_id": "run-1", "final_attempt_id": "a02",
|
||||
"attempts": [{"attempt_id": "a01", "harness_id": "old-route",
|
||||
"observed_model": "old-model", "profile_id": "old-profile"},
|
||||
{"attempt_id": "a02", **observed}],
|
||||
}), encoding="utf-8")
|
||||
return {**summary, "runDir": str(final.parent)}
|
||||
|
||||
|
||||
def _settled_run(tmp_path, monkeypatch, summary, observed=None):
|
||||
"""Drive a real `_settle` for `summary`; return (agent payload, ledger row, envelope).
|
||||
|
||||
The `delegate_run_settled` envelope is returned too because it RE-DERIVES the row's
|
||||
|
|
@ -208,6 +253,7 @@ def _settled_run(tmp_path, monkeypatch, summary):
|
|||
from ouroboros.gateways import claudexor as gw
|
||||
from ouroboros.tools.registry import ToolContext
|
||||
|
||||
summary = _observed_summary(tmp_path, summary, observed)
|
||||
class _Stub:
|
||||
def handshake(self, **_kw): return {}
|
||||
def get_run(self, rid, **_kw): return {"lastSeq": 9, "summary": dict(summary)}
|
||||
|
|
@ -232,7 +278,7 @@ def _settled_run(tmp_path, monkeypatch, summary):
|
|||
next(e for e in events if e.get("type") == "delegate_run_settled"))
|
||||
|
||||
|
||||
def _waited_run(tmp_path, monkeypatch, summary, requested_model="m"):
|
||||
def _waited_run(tmp_path, monkeypatch, summary, requested_model="m", observed=None):
|
||||
"""Drive one terminal `delegate_wait` for `summary`; return the agent payload.
|
||||
|
||||
Same transport walk as `_settled_run`, with the custody row's REQUESTED
|
||||
|
|
@ -242,6 +288,7 @@ def _waited_run(tmp_path, monkeypatch, summary, requested_model="m"):
|
|||
from ouroboros.gateways import claudexor as gw
|
||||
from ouroboros.tools.registry import ToolContext
|
||||
|
||||
summary = _observed_summary(tmp_path, summary, observed)
|
||||
class _Stub:
|
||||
def handshake(self, **_kw): return {}
|
||||
def get_run(self, rid, **_kw): return {"lastSeq": 9, "summary": dict(summary)}
|
||||
|
|
|
|||
|
|
@ -163,17 +163,71 @@ def test_schema_is_not_asked_on_an_interactive_transport_route(tmp_path, fake_ro
|
|||
|
||||
|
||||
def test_a_run_reported_off_the_pinned_route_is_disclosed(tmp_path, fake_route):
|
||||
"""Belt over the pin: the engine's own receipt of the pool the run used
|
||||
must echo the pinned route; drift surfaces as a capability_delta, never as
|
||||
a quietly accepted substitute route."""
|
||||
"""The final attempt, not the requested pool, witnesses route drift."""
|
||||
fake_route.detail = _terminal_detail('{"findings": []}', conformance="passed")
|
||||
fake_route.detail["summary"]["harnesses"] = ["other-route"]
|
||||
fake_route.detail["summary"]["harnesses"] = ["fake-review"]
|
||||
fake_route.telemetry = {"run_id": "run-1", "final_attempt_id": "a01", "attempts": [{
|
||||
"attempt_id": "a01", "harness_id": "other-route", "observed_model": "fake-small",
|
||||
}]}
|
||||
result = run_review_request(_agent_request(), slots=[_agent_slot()],
|
||||
drive_root=tmp_path, llm=FakeLLM())
|
||||
deltas = result.actors[0]["usage"]["capability_delta"]
|
||||
assert any(d["reason"] == "session_ran_off_pinned_route" for d in deltas)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("final_model", ["observed-model", None])
|
||||
def test_final_attempt_identity_reaches_review_projection_without_request_or_prior_fallback(
|
||||
tmp_path, monkeypatch, fake_route, final_model,
|
||||
):
|
||||
from ouroboros.reviewer_slot_config import record_reviewer_slot_executions, reviewer_slot_last_executions
|
||||
from ouroboros.review_execution_projection import review_executions_from_actor_usage
|
||||
|
||||
monkeypatch.setattr("ouroboros.config.DATA_DIR", tmp_path / "canonical-data")
|
||||
fake_route.detail = _terminal_detail('[]', model="request-model")
|
||||
fake_route.detail["summary"].update(
|
||||
harnesses=["fake-review"], authRoute={"profileId": "prior-profile"}, effectiveAccess="readonly",
|
||||
route={"observedModel": "prior-model", "harnessId": "prior-route", "verified": True},
|
||||
)
|
||||
fake_route.telemetry = {"run_id": "run-1", "final_attempt_id": "a02", "attempts": [
|
||||
{"attempt_id": "a01", "harness_id": "prior-route", "observed_model": "prior-model",
|
||||
"profile_id": "prior-profile"},
|
||||
{"attempt_id": "a02", "harness_id": "actual-route", "observed_model": final_model,
|
||||
"profile_id": "actual-profile"},
|
||||
]}
|
||||
slot = _agent_slot()
|
||||
result = run_review_request(_agent_request(), slots=[slot], drive_root=tmp_path, llm=FakeLLM())
|
||||
actor = result.actors[0]
|
||||
usage = actor["usage"]
|
||||
assert usage["resolved_model"] == (final_model or "")
|
||||
assert usage["delegated_route"] == "actual-route"
|
||||
assert usage["requested_route"] == "fake-review"
|
||||
assert usage["applied_profile"] == "actual-profile"
|
||||
assert usage["observed_attempt"]["attempt_id"] == "a02"
|
||||
assert len(fake_route.instances[0].start_requests) == 1
|
||||
assert review_executions_from_actor_usage([actor]) == [{
|
||||
"kind": "harness", "harness_id": "actual-route",
|
||||
**({"model": final_model} if final_model else {}),
|
||||
}]
|
||||
record_reviewer_slot_executions("scope_review", [SimpleNamespace(**actor)], {slot.slot_id: slot})
|
||||
row = reviewer_slot_last_executions()[slot.slot_id]
|
||||
assert row["effective"]["route"] == "agent_session:actual-route"
|
||||
assert row["effective"]["model"] == (final_model or "")
|
||||
assert row["effective"]["profile_id"] == "actual-profile"
|
||||
assert row["requested"]["model"] == slot.model
|
||||
|
||||
|
||||
def test_session_without_final_attempt_receipt_never_masquerades_as_api(tmp_path, fake_route):
|
||||
from ouroboros.review_execution_projection import review_executions_from_actor_usage
|
||||
|
||||
fake_route.telemetry = {"run_id": "run-1", "final_attempt_id": "missing", "attempts": []}
|
||||
result = run_review_request(_agent_request(), slots=[_agent_slot()], drive_root=tmp_path, llm=FakeLLM())
|
||||
actor = result.actors[0]
|
||||
assert actor["usage"]["resolved_model"] == ""
|
||||
assert actor["usage"]["delegated_route"] == ""
|
||||
assert actor["usage"]["observed_attempt"] == {}
|
||||
assert review_executions_from_actor_usage([actor]) == [{"kind": "harness"}]
|
||||
|
||||
|
||||
def test_conformance_gate_is_the_gate_not_run_success(tmp_path, fake_route):
|
||||
"""A successful run WITHOUT outputConformance == passed is narrative: the
|
||||
strict parser judges it, never the run's success."""
|
||||
|
|
@ -417,7 +471,7 @@ def test_positive_custody_session_failure_emits_one_unknown_cost_usage_row(tmp_p
|
|||
|
||||
assert len(rows) == 1
|
||||
assert rows[0]["provider"] == "claudexor"
|
||||
assert rows[0]["resolved_model"] == "api/model-a"
|
||||
assert rows[0]["resolved_model"] == "" # the requested model is not an execution receipt
|
||||
assert rows[0]["delegated_run_started"] is True
|
||||
assert rows[0]["delegated_run_id"] == "run-paid-failure"
|
||||
assert rows[0]["cost"] is None
|
||||
|
|
|
|||
|
|
@ -1,489 +1,42 @@
|
|||
"""Regression tests for review_execution._cross_check_findings.
|
||||
"""Preserve the contributor's disputed-finding examples without a host judge.
|
||||
|
||||
Closes ibl-e8665b941f7e — the triad reviewer hallucinated critical findings
|
||||
twice this cycle, blocking commit_reviewed without a real basis:
|
||||
|
||||
1. task acfaf5dc: triad claimed ``tests/test_attachment_staging.py`` imports
|
||||
``'ouroborosproject_naming'`` (missing dot). The real import is
|
||||
``'ouroboros.project_naming'`` — pytest collect confirmed 29 items.
|
||||
|
||||
2. task b924c25544ac4f61: triad claimed a ``'Claudexor.unavailable'`` typo.
|
||||
The codebase uses ``'ClaudexorUnavailable'`` (no dot) in 194 references.
|
||||
|
||||
These tests prove the cross-check catches BOTH shapes without widening
|
||||
to false-positive downgrades on real critical findings.
|
||||
The original PR cited ouroborosproject_naming and Claudexor.unavailable.
|
||||
Identifier absence alone cannot decide whether a missing-import finding is
|
||||
true. Canonicalization keeps each judgment intact for the existing rebuttal
|
||||
and review flow, on schema, strict, and extracted deliveries alike.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
from typing import Any, Dict, List
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from ouroboros.review_execution import ( # noqa: E402
|
||||
canonicalize_session_verdict,
|
||||
_cross_check_findings,
|
||||
_identifier_present_in_repo,
|
||||
)
|
||||
from ouroboros.review_verdict_extraction import canonicalize_session_verdict
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fixtures
|
||||
# ---------------------------------------------------------------------------
|
||||
@pytest.mark.parametrize("reason", [
|
||||
"tests/test_attachment_staging.py imports `ouroborosproject_naming` which does not exist.",
|
||||
"claudexor_daemon.py:1234 uses `Claudexor.unavailable` — typo.",
|
||||
"module `ouroboros.config` is missing required setup.",
|
||||
])
|
||||
@pytest.mark.parametrize("method", ["schema", "strict", "light_model_extraction"])
|
||||
def test_disputed_findings_keep_the_reviewers_verdict_and_severity(reason, method):
|
||||
findings = [{"item": "missing import", "verdict": "FAIL",
|
||||
"severity": "critical", "reason": reason}]
|
||||
calls = []
|
||||
|
||||
@pytest.fixture()
|
||||
def fake_repo(tmp_path: Path) -> Path:
|
||||
"""A self-contained repo tree with deliberate marker files.
|
||||
def chat(**kwargs):
|
||||
calls.append(kwargs)
|
||||
return {"content": json.dumps(findings)}, {"cost": 0.001}
|
||||
|
||||
The cross-check walks the tree to verify identifier presence. Using
|
||||
``tmp_path`` isolates the tests from the real ``/opt/ouroboros`` repo —
|
||||
so the assertions don't depend on the live tree's contents and the
|
||||
tests run identically regardless of working directory.
|
||||
"""
|
||||
repo = tmp_path / "fake_repo"
|
||||
repo.mkdir()
|
||||
# Real module marker (used by the positive control tests).
|
||||
(repo / "ouroboros").mkdir()
|
||||
(repo / "ouroboros" / "project_naming.py").write_text(
|
||||
"# real module marker\nPROJECT_NAME = 'ouroboros'\n",
|
||||
encoding="utf-8",
|
||||
raw = (json.dumps({"findings": findings}) if method == "schema" else
|
||||
json.dumps(findings) if method == "strict" else "Review completed. " + reason)
|
||||
text, actual_method, usage = canonicalize_session_verdict(
|
||||
raw, conformance_passed=method == "schema", llm=SimpleNamespace(chat=chat),
|
||||
)
|
||||
(repo / "ouroboros" / "config.py").write_text(
|
||||
"# real config module\nSETTINGS = {}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
# Real class/identifier marker — ``ClaudexorUnavailable`` appears here.
|
||||
(repo / "claudexor_daemon.py").write_text(
|
||||
"class ClaudexorUnavailable(Exception):\n pass\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
# Hallucinated identifier markers — these names NEVER appear in the
|
||||
# fake repo, so any finding mentioning only them is a clean test target.
|
||||
(repo / "tests").mkdir()
|
||||
(repo / "tests" / "test_attachment_staging.py").write_text(
|
||||
"from ouroboros.project_naming import PROJECT_NAME\n"
|
||||
"def test_staging_smoke():\n assert PROJECT_NAME\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
# Vendor/cache trees that the walker must skip.
|
||||
(repo / "__pycache__").mkdir()
|
||||
(repo / "__pycache__" / "hallucinated_fabricated.py").write_text(
|
||||
"fabricated_ouroborosproject_naming = True\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
return repo
|
||||
|
||||
|
||||
def _critical(
|
||||
*,
|
||||
item: str = "missing import",
|
||||
reason: str,
|
||||
severity: str = "critical",
|
||||
) -> Dict[str, Any]:
|
||||
"""A minimal critical finding shape that mirrors the canonical contract."""
|
||||
return {
|
||||
"item": item,
|
||||
"verdict": "FAIL",
|
||||
"severity": severity,
|
||||
"reason": reason,
|
||||
}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Counter-existence — proves the test infra itself works.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestIdentifierResolution:
|
||||
"""Sanity tests for the verification primitive itself."""
|
||||
|
||||
def test_real_dotted_module_resolves_via_filesystem(
|
||||
self, fake_repo: Path,
|
||||
) -> None:
|
||||
# ouroboros.project_naming → repo/ouroboros/project_naming.py exists.
|
||||
assert _identifier_present_in_repo(
|
||||
"ouroboros.project_naming", fake_repo,
|
||||
)
|
||||
|
||||
def test_hallucinated_dotted_module_is_absent(self, fake_repo: Path) -> None:
|
||||
# ouroborosproject_naming has NO dot, so the filesystem fast path
|
||||
# treats it as a single token (no split, nothing to resolve) and
|
||||
# the substring walker finds zero matches in real files (the
|
||||
# __pycache__ copy is excluded by the SKIP_DIRS filter).
|
||||
assert not _identifier_present_in_repo(
|
||||
"ouroborosproject_naming", fake_repo,
|
||||
)
|
||||
|
||||
def test_real_camelcase_identifier_resolves_via_substring_walk(
|
||||
self, fake_repo: Path,
|
||||
) -> None:
|
||||
# ``ClaudexorUnavailable`` is NOT a dotted module (no dot, so the
|
||||
# fast path skips). The substring walker finds it in
|
||||
# ``claudexor_daemon.py``. This proves the CamelCase catch-all
|
||||
# catches the real identifier even when fast-path fails.
|
||||
assert _identifier_present_in_repo(
|
||||
"ClaudexorUnavailable", fake_repo,
|
||||
)
|
||||
|
||||
def test_hallucinated_dotted_classname_is_absent_via_walk(
|
||||
self, fake_repo: Path,
|
||||
) -> None:
|
||||
# ``Claudexor.unavailable`` splits on the dot into ``Claudexor``
|
||||
# and ``unavailable``. Neither resolves as a module path.
|
||||
# ``Claudexor`` alone (CamelCase) appears in the file ONLY as part
|
||||
# of the joined ``ClaudexorUnavailable`` token; the substring
|
||||
# walker matches substring, so ``Claudexor`` IS found there. But
|
||||
# the FULL dotted token ``Claudexor.unavailable`` is not a
|
||||
# substring of ``ClaudexorUnavailable`` (the dot breaks the join).
|
||||
# Therefore the check on the dotted form returns False.
|
||||
assert not _identifier_present_in_repo(
|
||||
"Claudexor.unavailable", fake_repo,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Confirmed hallucination cases — ibl-e8665b941f7e regression set
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestCrossCheckDowngradesHallucinatedFindings:
|
||||
"""Both confirmed hallucination cases must downgrade critical → advisory."""
|
||||
|
||||
def test_case_1_fabricated_import_downgrades(self, fake_repo: Path) -> None:
|
||||
"""task acfaf5dc: triad cited ``'ouroborosproject_naming'``."""
|
||||
findings: List[Dict[str, Any]] = [_critical(
|
||||
item="attachment_staging_import",
|
||||
reason=(
|
||||
"tests/test_attachment_staging.py imports `ouroborosproject_naming` "
|
||||
"which does not exist in this repository."
|
||||
),
|
||||
)]
|
||||
out, audit = _cross_check_findings(findings, fake_repo)
|
||||
assert len(out) == 1
|
||||
assert out[0]["severity"] == "advisory"
|
||||
assert "cross-check" in out[0]["reason"].lower()
|
||||
assert audit["checked"] == 1
|
||||
assert audit["downgraded"] == 1
|
||||
assert audit["kept"] == 0
|
||||
# The audit entry names the absent identifier verbatim.
|
||||
assert any(
|
||||
"ouroborosproject_naming" in entry.get("absent_identifiers", [])
|
||||
for entry in audit["entries"]
|
||||
)
|
||||
|
||||
def test_case_2_fabricated_typo_classname_downgrades(
|
||||
self, fake_repo: Path,
|
||||
) -> None:
|
||||
"""task b924c25544ac4f61: triad cited ``'Claudexor.unavailable'`` typo."""
|
||||
findings: List[Dict[str, Any]] = [_critical(
|
||||
item="claudexor_daemon_typo",
|
||||
reason=(
|
||||
"claudexor_daemon.py:1234 uses `Claudexor.unavailable` — typo."
|
||||
),
|
||||
)]
|
||||
out, audit = _cross_check_findings(findings, fake_repo)
|
||||
assert len(out) == 1
|
||||
assert out[0]["severity"] == "advisory"
|
||||
assert audit["downgraded"] == 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Defensive — never widen the downgrade scope past what the reviewer asserted
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestCrossCheckDefensiveBoundaries:
|
||||
"""Critical findings that ARE substantiated must stay critical."""
|
||||
|
||||
def test_real_identifier_stays_critical(self, fake_repo: Path) -> None:
|
||||
findings: List[Dict[str, Any]] = [_critical(
|
||||
item="config_import_failure",
|
||||
reason=(
|
||||
"module `ouroboros.config` is missing the required `x` setting; "
|
||||
"this should be present."
|
||||
),
|
||||
)]
|
||||
out, audit = _cross_check_findings(findings, fake_repo)
|
||||
assert len(out) == 1
|
||||
assert out[0]["severity"] == "critical"
|
||||
assert audit["checked"] == 1
|
||||
assert audit["downgraded"] == 0
|
||||
assert audit["kept"] == 1
|
||||
|
||||
def test_partial_identifier_match_keeps_critical(
|
||||
self, fake_repo: Path,
|
||||
) -> None:
|
||||
"""When SOME identifiers exist and others don't, do NOT downgrade.
|
||||
|
||||
A partial match can mean the reviewer's claim is partly true;
|
||||
downgrading here would silently unblock an actually-broken part
|
||||
of the codebase.
|
||||
"""
|
||||
findings: List[Dict[str, Any]] = [_critical(
|
||||
item="mixed_claim",
|
||||
reason=(
|
||||
"Compare `ouroboros.config` (real, present) with the "
|
||||
"`ouroboros.does_not_exist_helper` (fake) — the helper "
|
||||
"is missing from the codebase, expected to exist there."
|
||||
),
|
||||
)]
|
||||
out, audit = _cross_check_findings(findings, fake_repo)
|
||||
assert len(out) == 1
|
||||
assert out[0]["severity"] == "critical" # conservative
|
||||
assert audit["downgraded"] == 0
|
||||
|
||||
def test_vague_critical_with_no_identifiers_kept(
|
||||
self, fake_repo: Path,
|
||||
) -> None:
|
||||
"""A critical that names no concrete identifier is left untouched.
|
||||
|
||||
The cross-check is intentionally narrow. A vague FAIL without
|
||||
code-shape tokens has nothing to verify against.
|
||||
"""
|
||||
findings: List[Dict[str, Any]] = [_critical(
|
||||
item="vague_architecture",
|
||||
reason=(
|
||||
"FAIL: the overall architecture does not match the proposed design."
|
||||
),
|
||||
)]
|
||||
out, audit = _cross_check_findings(findings, fake_repo)
|
||||
assert len(out) == 1
|
||||
assert out[0]["severity"] == "critical"
|
||||
assert audit["checked"] == 1
|
||||
assert audit["downgraded"] == 0
|
||||
assert audit["kept"] == 1
|
||||
|
||||
def test_advisory_severity_is_never_modified(self, fake_repo: Path) -> None:
|
||||
"""Advisory findings pass through unchanged — they're not blocking."""
|
||||
findings: List[Dict[str, Any]] = [_critical(
|
||||
item="advisory_naming",
|
||||
severity="advisory",
|
||||
reason="`ouroborosproject_naming` is referenced; double check spelling.",
|
||||
)]
|
||||
out, audit = _cross_check_findings(findings, fake_repo)
|
||||
assert len(out) == 1
|
||||
assert out[0]["severity"] == "advisory" # unchanged
|
||||
assert audit["checked"] == 0 # only critical findings are checked
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Fail-closed — per-file OSError must not silently produce a downgrade
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestFailClosedOnPerFileOSError:
|
||||
"""self_consistency (triad finding ibl-e8665b941f7e follow-up).
|
||||
|
||||
A partial walk where ANY per-file ``OSError`` occurred (on ``stat`` or
|
||||
``open``) must NOT claim ``identifier == absent``. The unreadable file
|
||||
might be the one containing the identifier, so we conservatively keep
|
||||
the finding critical. The cross-check is fail-closed: incomplete
|
||||
evidence == no downgrade.
|
||||
"""
|
||||
|
||||
def test_open_oserror_on_real_file_fails_closed(
|
||||
self, fake_repo: Path, monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""If ``open()`` raises OSError on a real file, fail-closed: True.
|
||||
|
||||
The identifier ``ouroborosproject_naming`` is genuinely absent from
|
||||
the repo (the only file referencing it is inside ``__pycache__``,
|
||||
which the walker skips). Without OSError, the function returns
|
||||
``False`` (absent). WITH OSError on one file open during the walk,
|
||||
the function MUST return ``True`` (fail-closed) so we do not
|
||||
silently downgrade a critical finding based on incomplete
|
||||
evidence.
|
||||
"""
|
||||
real_open = open
|
||||
failing_path = str(fake_repo / "claudexor_daemon.py")
|
||||
|
||||
def patched_open(file, *args, **kwargs):
|
||||
if str(file) == failing_path:
|
||||
raise OSError("simulated permission denied on open")
|
||||
return real_open(file, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr("builtins.open", patched_open)
|
||||
|
||||
# Genuinely absent identifier, but the walk hits an OSError.
|
||||
# Fail-closed must return True (treat as present).
|
||||
assert _identifier_present_in_repo(
|
||||
"ouroborosproject_naming", fake_repo,
|
||||
) is True
|
||||
|
||||
def test_stat_oserror_on_real_file_fails_closed(
|
||||
self, fake_repo: Path, monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""If ``stat()`` raises OSError on a real file, fail-closed: True.
|
||||
|
||||
Same fail-closed contract applies when the per-file ``stat()`` call
|
||||
(size check before reading) raises OSError — that file is the one
|
||||
we cannot prove does NOT contain the identifier.
|
||||
"""
|
||||
import pathlib
|
||||
|
||||
real_stat = pathlib.Path.stat
|
||||
failing_path = str(fake_repo / "claudexor_daemon.py")
|
||||
|
||||
def patched_stat(self, *args, **kwargs):
|
||||
if str(self) == failing_path:
|
||||
raise OSError("simulated permission denied on stat")
|
||||
return real_stat(self, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(pathlib.Path, "stat", patched_stat)
|
||||
|
||||
# Identifier is absent from readable files, but stat on one file
|
||||
# raised OSError. Fail-closed must return True.
|
||||
assert _identifier_present_in_repo(
|
||||
"ouroborosproject_naming", fake_repo,
|
||||
) is True
|
||||
|
||||
def test_walk_failure_without_oserror_still_returns_absent(
|
||||
self, fake_repo: Path,
|
||||
) -> None:
|
||||
"""Negative control: no OSError, no OSError tracker set, returns False.
|
||||
|
||||
When the walker completes successfully and the identifier is NOT
|
||||
in any file, the function returns ``False`` (absent). The
|
||||
fail-closed tracker must not over-trigger on the clean path.
|
||||
"""
|
||||
# Without any OSError, ``ouroborosproject_naming`` is absent from
|
||||
# the readable files (the ``__pycache__`` copy is skipped by
|
||||
# SKIP_DIRS), so the function returns False — not True.
|
||||
assert _identifier_present_in_repo(
|
||||
"ouroborosproject_naming", fake_repo,
|
||||
) is False
|
||||
|
||||
def test_real_identifier_still_found_with_clean_walk(
|
||||
self, fake_repo: Path,
|
||||
) -> None:
|
||||
"""Positive control: real identifier, clean walk, returns True.
|
||||
|
||||
The fail-closed tracker must not break the happy path: when the
|
||||
identifier IS in a readable file, we still return True (without
|
||||
needing to scan the rest of the tree).
|
||||
"""
|
||||
assert _identifier_present_in_repo(
|
||||
"ClaudexorUnavailable", fake_repo,
|
||||
) is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Off-by-default — existing call sites see no behavioral change
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestCrossCheckOptIn:
|
||||
def test_no_repo_root_means_passthrough(self) -> None:
|
||||
findings: List[Dict[str, Any]] = [_critical(
|
||||
item="fabricated_critical",
|
||||
reason="`ouroborosproject_naming` should exist",
|
||||
)]
|
||||
out, audit = _cross_check_findings(findings, None)
|
||||
assert out == findings
|
||||
assert audit == {"checked": 0, "downgraded": 0, "kept": 0, "entries": []}
|
||||
|
||||
def test_empty_findings_returns_empty_audit(self, fake_repo: Path) -> None:
|
||||
out, audit = _cross_check_findings([], fake_repo)
|
||||
assert out == []
|
||||
assert audit == {"checked": 0, "downgraded": 0, "kept": 0, "entries": []}
|
||||
|
||||
def test_nonexistent_repo_root_defaults_to_passthrough(
|
||||
self, tmp_path: Path,
|
||||
) -> None:
|
||||
findings: List[Dict[str, Any]] = [_critical(
|
||||
item="fabricated_critical",
|
||||
reason="`ouroborosproject_naming` should exist",
|
||||
)]
|
||||
out, audit = _cross_check_findings(findings, tmp_path / "does_not_exist")
|
||||
assert out == findings
|
||||
assert audit["checked"] == 0
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Integration — cross-check inside canonicalize_session_verdict
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
class TestCanonicalizeSessionVerdictCrossCheck:
|
||||
"""The cross-check must run inside canonicalize_session_verdict, not only
|
||||
at the helper layer, so all three parse paths (schema / strict /
|
||||
light_model_extraction) inherit the protection."""
|
||||
|
||||
def test_schema_path_downgrades_hallucinated_critical(
|
||||
self, fake_repo: Path,
|
||||
) -> None:
|
||||
raw = json.dumps({
|
||||
"findings": [{
|
||||
"item": "fabricated_import",
|
||||
"verdict": "FAIL",
|
||||
"severity": "critical",
|
||||
"reason": "imports `ouroborosproject_naming` — that doesn't exist",
|
||||
}],
|
||||
})
|
||||
text, method, usage = canonicalize_session_verdict(
|
||||
raw,
|
||||
conformance_passed=True,
|
||||
repo_root=str(fake_repo),
|
||||
)
|
||||
assert method == "schema"
|
||||
canonical_findings = json.loads(text)
|
||||
assert canonical_findings[0]["severity"] == "advisory"
|
||||
assert "cross-check" in canonical_findings[0]["reason"].lower()
|
||||
assert usage.get("cross_check", {}).get("downgraded") == 1
|
||||
|
||||
def test_strict_path_downgrades_hallucinated_critical(
|
||||
self, fake_repo: Path,
|
||||
) -> None:
|
||||
# The strict path takes the WHOLE text as the findings array —
|
||||
# produce exactly that shape (a bare list, not wrapped in
|
||||
# ``{"findings": ...}``).
|
||||
raw = json.dumps([{
|
||||
"item": "fabricated_typo",
|
||||
"verdict": "FAIL",
|
||||
"severity": "critical",
|
||||
"reason": "typo: `Claudexor.unavailable`",
|
||||
}])
|
||||
text, method, usage = canonicalize_session_verdict(
|
||||
raw,
|
||||
conformance_passed=False,
|
||||
repo_root=str(fake_repo),
|
||||
)
|
||||
assert method == "strict"
|
||||
canonical_findings = json.loads(text)
|
||||
assert canonical_findings[0]["severity"] == "advisory"
|
||||
assert usage.get("cross_check", {}).get("downgraded") == 1
|
||||
|
||||
def test_repo_root_none_keeps_existing_behavior(self, fake_repo: Path) -> None:
|
||||
"""When repo_root is None, critical findings pass through unchanged."""
|
||||
raw = json.dumps([{
|
||||
"item": "fabricated_import",
|
||||
"verdict": "FAIL",
|
||||
"severity": "critical",
|
||||
"reason": "imports `ouroborosproject_naming` — that doesn't exist",
|
||||
}])
|
||||
text, _method, usage = canonicalize_session_verdict(
|
||||
raw,
|
||||
conformance_passed=False,
|
||||
repo_root=None,
|
||||
)
|
||||
canonical_findings = json.loads(text)
|
||||
assert canonical_findings[0]["severity"] == "critical"
|
||||
assert "cross_check" not in usage # audit only recorded when active
|
||||
|
||||
def test_real_critical_stays_critical_through_canonicalize(
|
||||
self, fake_repo: Path,
|
||||
) -> None:
|
||||
raw = json.dumps({
|
||||
"findings": [{
|
||||
"item": "real_issue",
|
||||
"verdict": "FAIL",
|
||||
"severity": "critical",
|
||||
"reason": (
|
||||
"module `ouroboros.config` is missing required setup; "
|
||||
"expected to contain configuration."
|
||||
),
|
||||
}],
|
||||
})
|
||||
text, _method, _usage = canonicalize_session_verdict(
|
||||
raw,
|
||||
conformance_passed=True,
|
||||
repo_root=str(fake_repo),
|
||||
)
|
||||
canonical_findings = json.loads(text)
|
||||
assert canonical_findings[0]["severity"] == "critical"
|
||||
assert actual_method == method
|
||||
assert json.loads(text) == findings
|
||||
assert "cross_check" not in usage
|
||||
assert len(calls) == (1 if method == "light_model_extraction" else 0)
|
||||
if method == "strict":
|
||||
assert text == raw
|
||||
|
|
|
|||
|
|
@ -1,108 +1,31 @@
|
|||
"""The cross_check audit that ``canonicalize_session_verdict`` emits must reach
|
||||
the persisted usage for EVERY applicable verdict method (schema / strict /
|
||||
light_model_extraction), not only the light branch where it used to live under
|
||||
``usage['extraction']['cross_check']``. Schema and strict silently dropped the
|
||||
audit entry, breaking the audit trail.
|
||||
"""
|
||||
"""The original disputed finding and transcript survive session projection."""
|
||||
|
||||
import json
|
||||
|
||||
import pytest
|
||||
|
||||
# _fake_repo spawns real `git` subprocesses (CONTRIBUTING §4).
|
||||
pytestmark = pytest.mark.serial
|
||||
|
||||
@pytest.mark.parametrize("conformed", [True, False])
|
||||
def test_verdict_result_keeps_disputed_finding_and_full_raw_source(tmp_path, conformed):
|
||||
from ouroboros.review_execution import AgentSessionReviewExecutor, ReviewAssignment, ReviewRouteKind
|
||||
from ouroboros.review_substrate import ReviewRequest, ReviewSlot
|
||||
|
||||
def _fake_repo(tmp_path):
|
||||
"""Tiny git repo on disk so _cross_check_findings has ground truth to
|
||||
verify hallucinated import claims against. Inline copy of the fixture
|
||||
from test_review_cross_check.py because fixtures don't cross files."""
|
||||
import subprocess
|
||||
repo = tmp_path / "fake_repo"
|
||||
repo.mkdir()
|
||||
pkg = repo / "ouroboros"
|
||||
pkg.mkdir()
|
||||
(pkg / "__init__.py").write_text("")
|
||||
(pkg / "real_module.py").write_text("def real_call():\n return 1\n")
|
||||
subprocess.run(["git", "-C", str(repo), "init", "-q"], check=True)
|
||||
subprocess.run(["git", "-C", str(repo), "config", "user.email", "t@t"], check=True)
|
||||
subprocess.run(["git", "-C", str(repo), "config", "user.name", "t"], check=True)
|
||||
subprocess.run(["git", "-C", str(repo), "add", "-A"], check=True)
|
||||
subprocess.run(["git", "-C", str(repo), "commit", "-q", "-m", "init"], check=True)
|
||||
return repo
|
||||
findings = [{"item": "fabricated_import_x1", "verdict": "FAIL",
|
||||
"severity": "critical", "reason": "imports `ouroborosproject_x1`"}]
|
||||
raw = json.dumps({"findings": findings} if conformed else findings)
|
||||
request = ReviewRequest(surface="scope_review", goal="Review the staged change.",
|
||||
task_id="disputed-finding", session_root=str(tmp_path))
|
||||
slot = ReviewSlot(slot_id="slot_x", model="api/model-a", timeout_sec=30,
|
||||
route=ReviewRouteKind.AGENT_SESSION)
|
||||
executor = AgentSessionReviewExecutor(ReviewAssignment(request, slot, "call-x"), llm=None)
|
||||
executor._session_usage = {}
|
||||
executor._deltas = []
|
||||
executor._raw_transcript = raw
|
||||
executor._conformance_passed = conformed
|
||||
|
||||
result = executor._verdict_result()
|
||||
|
||||
class TestVerdictResultCrossCheckPersistence:
|
||||
"""End-to-end through _verdict_result: the cross_check audit entry that
|
||||
canonicalize_session_verdict emits must reach the persisted usage for
|
||||
every applicable method, not only light_model_extraction."""
|
||||
|
||||
def _executor(self, tmp_path, fake_repo):
|
||||
from ouroboros.review_execution import AgentSessionReviewExecutor
|
||||
from ouroboros.review_substrate import (
|
||||
ReviewAssignment,
|
||||
ReviewRequest,
|
||||
ReviewRouteKind,
|
||||
ReviewSlot,
|
||||
)
|
||||
|
||||
request = ReviewRequest(
|
||||
surface="scope_review",
|
||||
goal="Review the staged change.",
|
||||
task_id="t-ibl-01b310c0ce18",
|
||||
call_type="scope_review",
|
||||
session_root=str(fake_repo),
|
||||
session_task="Review the staged diff.",
|
||||
)
|
||||
slot = ReviewSlot(
|
||||
slot_id="slot_x",
|
||||
model="api/model-a",
|
||||
timeout_sec=30,
|
||||
route=ReviewRouteKind.AGENT_SESSION,
|
||||
)
|
||||
assignment = ReviewAssignment(
|
||||
request=request,
|
||||
slot=slot,
|
||||
call_id="call-x",
|
||||
call_type="scope_review",
|
||||
)
|
||||
executor = AgentSessionReviewExecutor(assignment, llm=None)
|
||||
executor._session_usage = {}
|
||||
executor._deltas = []
|
||||
return executor
|
||||
|
||||
def test_schema_path_persists_cross_check_audit(self, tmp_path):
|
||||
"""Schema branch: conformance passed, hallucinated import claims must
|
||||
trigger downgrades, and the audit entry must reach result.usage."""
|
||||
text = (
|
||||
'{"findings": [{"item": "fabricated_import_x1", "verdict": "FAIL", '
|
||||
'"severity": "critical", "reason": "imports `ouroborosproject_x1`"}]}'
|
||||
)
|
||||
executor = self._executor(tmp_path, _fake_repo(tmp_path))
|
||||
executor._raw_transcript = text
|
||||
executor._conformance_passed = True
|
||||
|
||||
result = executor._verdict_result()
|
||||
|
||||
assert result.usage["verdict_method"] == "schema"
|
||||
assert "cross_check" in result.usage, (
|
||||
"schema branch dropped the cross_check audit — ibl-01b310c0ce18"
|
||||
)
|
||||
assert result.usage["cross_check"]["downgraded"] == 1
|
||||
|
||||
def test_strict_path_persists_cross_check_audit(self, tmp_path):
|
||||
"""Strict branch: conformance fails, whole text is a parseable array;
|
||||
same audit-entry preservation requirement."""
|
||||
text = (
|
||||
'[{"item": "fabricated_import_x2", "verdict": "FAIL", '
|
||||
'"severity": "critical", "reason": "imports `ouroborosproject_x2`"}]'
|
||||
)
|
||||
executor = self._executor(tmp_path, _fake_repo(tmp_path))
|
||||
executor._raw_transcript = text
|
||||
executor._conformance_passed = False
|
||||
|
||||
result = executor._verdict_result()
|
||||
|
||||
assert result.usage["verdict_method"] == "strict"
|
||||
assert "cross_check" in result.usage, (
|
||||
"strict branch dropped the cross_check audit — ibl-01b310c0ce18"
|
||||
)
|
||||
assert result.usage["cross_check"]["downgraded"] == 1
|
||||
assert json.loads(result.raw_text) == findings
|
||||
assert result.usage["verdict_method"] == ("schema" if conformed else "strict")
|
||||
assert result.message["session_transcript"] == raw
|
||||
assert "cross_check" not in result.usage
|
||||
|
|
|
|||
|
|
@ -868,14 +868,14 @@ def test_runs_as_records_applied_facts_never_requested_as_applied():
|
|||
|
||||
|
||||
def test_runner_facts_carry_the_applied_receipt_fields():
|
||||
"""The session runner surfaces authRoute.profileId/effectiveAccess from the
|
||||
summary — the one source settle_run (D29) reads too."""
|
||||
"""The session runner shares final-attempt identity with settlement;
|
||||
effectiveAccess remains an independent engine fact."""
|
||||
import inspect
|
||||
|
||||
from ouroboros import review_execution
|
||||
|
||||
source = inspect.getsource(review_execution.run_delegated_review_session)
|
||||
assert '"applied_profile"' in source and "authRoute" in source
|
||||
assert '"applied_profile"' in source and "final_attempt_facts" in source
|
||||
assert '"applied_access"' in source and "effectiveAccess" in source
|
||||
|
||||
|
||||
|
|
|
|||
108
tests/test_subscription_observation_replay.py
Normal file
108
tests/test_subscription_observation_replay.py
Normal file
|
|
@ -0,0 +1,108 @@
|
|||
"""One physical session remains one charge across late model observations."""
|
||||
|
||||
import json
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from ouroboros import delegate_custody as custody, usage_accounting as usage
|
||||
from tests import fixtures_usage_compaction as compaction
|
||||
|
||||
|
||||
def _record(root, model, **overrides):
|
||||
args = dict(route="selected-route", task_id="task", root_task_id="root", parent_task_id="parent",
|
||||
category="review", source="test", review_skill="skill", review_wave_id="wave",
|
||||
review_slot_id="slot", spend_usd=2.5)
|
||||
args.update(overrides)
|
||||
return usage.record_subscription_session("same-run", drive_root=root, model=model, **args)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("before_model,after_model", [("actual", ""), ("", "actual"), ("first", "second")])
|
||||
def test_later_model_observation_replays_the_original_row_without_repricing(tmp_path, before_model, after_model):
|
||||
first = _record(tmp_path, before_model)
|
||||
ledger = tmp_path / usage.LEDGER_REL
|
||||
before = ledger.read_bytes()
|
||||
assert _record(tmp_path, after_model, spend_usd=99) == first
|
||||
assert ledger.read_bytes() == before
|
||||
row = next(row for row in map(json.loads, before.splitlines()) if row.get("attempt_id") == first)
|
||||
assert row["model"] == before_model and row["cost_usd"] == 2.5
|
||||
|
||||
|
||||
@pytest.mark.parametrize("field", ["route", "task_id", "root_task_id", "parent_task_id", "category", "source",
|
||||
"review_skill", "review_wave_id", "review_slot_id"])
|
||||
def test_late_model_never_waives_remaining_caller_identity(tmp_path, field):
|
||||
_record(tmp_path, "first")
|
||||
ledger = tmp_path / usage.LEDGER_REL
|
||||
before = ledger.read_bytes()
|
||||
with pytest.raises(usage.UsageAccountingError, match="conflicting settled-row identity"):
|
||||
_record(tmp_path, "second", **{field: "different-owner-or-route"})
|
||||
assert ledger.read_bytes() == before
|
||||
|
||||
|
||||
@pytest.mark.parametrize("field,value", [("kind", "external_unmetered"), ("provider", "other"),
|
||||
("subscription_route", "other"), ("session_id_sha256", "0" * 64)])
|
||||
def test_derived_identity_fields_remain_checked(tmp_path, field, value):
|
||||
attempt_id = _record(tmp_path, "first")
|
||||
ledger = tmp_path / usage.LEDGER_REL
|
||||
rows = [json.loads(line) for line in ledger.read_text().splitlines()]
|
||||
# Inject a conflicting stored identity while retaining the same lookup id.
|
||||
next(row for row in rows if row.get("attempt_id") == attempt_id)[field] = value
|
||||
ledger.write_text("".join(json.dumps(row) + "\n" for row in rows), encoding="utf-8")
|
||||
before = ledger.read_bytes()
|
||||
with pytest.raises(usage.UsageAccountingError, match="conflicting settled-row identity"):
|
||||
_record(tmp_path, "second")
|
||||
assert ledger.read_bytes() == before
|
||||
|
||||
|
||||
def test_external_unmetered_keeps_its_existing_model_identity(tmp_path):
|
||||
usage.record_unmetered_external_dispatch("same-external", drive_root=tmp_path, model="first", task_id="task")
|
||||
before = (tmp_path / usage.LEDGER_REL).read_bytes()
|
||||
with pytest.raises(usage.UsageAccountingError, match="conflicting settled-row identity"):
|
||||
usage.record_unmetered_external_dispatch("same-external", drive_root=tmp_path, model="second", task_id="task")
|
||||
assert (tmp_path / usage.LEDGER_REL).read_bytes() == before
|
||||
|
||||
|
||||
data_root = compaction.data_root
|
||||
data_root_any_tier = compaction.data_root_any_tier
|
||||
compacted = compaction.compacted
|
||||
|
||||
|
||||
def test_model_observation_replay_after_compaction_keeps_the_retained_charge(data_root, compacted):
|
||||
before = (data_root / usage.LEDGER_REL).read_bytes()
|
||||
usage.record_subscription_session(
|
||||
"sess-1", drive_root=data_root, route="claudexor:claude", model="new-observation",
|
||||
task_id="t5", root_task_id="root", spend_usd=99,
|
||||
)
|
||||
assert (data_root / usage.LEDGER_REL).read_bytes() == before
|
||||
|
||||
|
||||
@pytest.mark.parametrize("observed_model", ["actual-model", None])
|
||||
def test_lost_custody_checkpoint_recovers_without_rewriting_the_old_charge(tmp_path, observed_model):
|
||||
drive = tmp_path / "data"
|
||||
entry = custody.RunCustody(run_id="run-replay", task_id="t", root_task_id="t",
|
||||
route_id="selected-route", model="requested-model")
|
||||
assert custody.record_started(drive, entry)
|
||||
usage.record_subscription_session("run-replay", drive_root=drive, route="selected-route",
|
||||
model="requested-model", task_id="t", root_task_id="t", spend_usd=2.5)
|
||||
resumed = custody.replay(drive)["run-replay"]
|
||||
assert not resumed.ledger_recorded and not resumed.settled
|
||||
final = tmp_path / "engine" / "final"
|
||||
final.mkdir(parents=True)
|
||||
(final / "telemetry.yaml").write_text(json.dumps({
|
||||
"run_id": "run-replay", "final_attempt_id": "a02", "attempts": [
|
||||
{"attempt_id": "a01", "observed_model": "requested-model"},
|
||||
{"attempt_id": "a02", "observed_model": observed_model,
|
||||
"harness_id": "selected-route", "profile_id": "profile"},
|
||||
]}), encoding="utf-8")
|
||||
detail = {"summary": {"runDir": str(final.parent), "state": "succeeded",
|
||||
"model": "requested-model", "spendUsd": 2.5}}
|
||||
ledger = drive / usage.LEDGER_REL
|
||||
before = ledger.read_bytes()
|
||||
result = custody.settle_run(drive, SimpleNamespace(), resumed, detail)
|
||||
assert result["settled"] and result["ledger_recorded"]
|
||||
assert ledger.read_bytes() == before
|
||||
rows = [json.loads(line) for line in (drive / "logs" / "events.jsonl").read_text().splitlines()]
|
||||
event = next(row for row in rows if row.get("type") == custody.SETTLED)
|
||||
assert event["model"] == (observed_model or "")
|
||||
assert event["observed_attempt"]["attempt_id"] == "a02"
|
||||
assert event["cost_usd"] == 2.5
|
||||
Loading…
Add table
Add a link
Reference in a new issue