diff --git a/CHANGELOG.md b/CHANGELOG.md index b22e7c1f..29a5b391 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,11 +10,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] ### Fixed +- **`Orchestrator.resume_branch_with_user_response`: a resumed user-interaction branch is now discarded from its barrier candidacy when terminated** (`coordination/execution/orchestrator.py`; Framework Session 16 / ADR-012). The seam terminated the suspended branch and spawned the resume_agent branch toward the same `delivery_target`, but never removed the now-terminated suspended branch from that barrier's candidate set — so `Barrier.pending()` (`candidates − arrived − failed`) kept listing it forever and the barrier never fired. Latent in the SYNC `ask_user` path too, uncaught because the SYNC handler tests exercise `UserNodeHandler` in isolation, never an orchestrator-level UserNode→resume→terminal run (the gap Session 03 flagged); surfaced by ADR-012's first such test. Fix mirrors `_deliver`'s existing cross-barrier cleanup — discard the terminated branch from every `candidate_of` barrier. +- **`Orchestra.resume_session`: the rebuilt EventBus now re-points the reused publishers (`step_executor`, user-node handler), so a consumer re-attached via `on_bus_rebuilt` receives the resumed dispatch's `LLMCallEvent`s** (`coordination/orchestra.py` `_wire_event_bus`; completes FW17 / ADR-011). `resume_session` rebuilds `self.event_bus` and `_wire_event_bus` re-creates the listener set (TraceCollector / StatusManager / AGGUITranslator) on it — but the REUSED `step_executor`, which emits `LLMCallEvent` (the event per-run cost is computed from), still published on the prior bus. So a per-run cost adapter re-attached via `on_bus_rebuilt` (FW17's motivating use case) received nothing on resume, and post-resume LLM spend was never billed. FW17's resume test missed it (stub agent, no LLM call — `on_bus_rebuilt` was only exercised against orchestrator-level `BranchCompletedEvent`); a real-model pause→resume (Spren `test_pause_resume_live.py`) surfaced it. Fix: `_wire_event_bus` re-points `step_executor.event_bus` + `_user_node_handler.event_bus` to `self.event_bus`, guarded (absent at `__init__`, where they bind the fresh bus at construction; present on resume). The 34-test pause/resume suite stays green. - **`RealRuntime`: parallel-invoke validation cross-talk race fixed; per-branch agent instance is threaded, not shared** (`coordination/execution/real_runtime.py`; issue #40). `RealRuntime` is constructed once per `Orchestra.run()` and the orchestrator dispatches `step()` for every runnable branch as a concurrent `asyncio.Task` on that one shared object. `step()` stashed the per-branch agent instance on `self._current_instance` *before* the `execute_step` await and `_translate()` read it back *after* the await to feed `ValidationProcessor.validate_coordination_action(agent=...)`. Under `parallel_invoke` fan-out a sibling branch overwrote the shared attribute during the await, so a branch's coordination action was validated against a **different** agent's identity — and thus its outgoing topology edges — producing a fabricated, misattributed `"Agent cannot invoke: [...]"` failure (latent when fanned-out workers shared identical outgoing edges; surfaced non-deterministically when they differed). A direct DP-004 (branch isolation) violation. Fix: thread the instance as an explicit parameter into `_translate` (`agent=instance`) and delete `self._current_instance`, so the shared runtime holds zero per-step mutable state — conforming the lone outlier to the file's existing convention (`execute_step`/`_build_content_only_diagnostic` already take `instance` explicitly). Public surface unchanged: the `Runtime` Protocol and `step(branch)` signature are untouched; only the private `_translate` signature changes. Regression guard: a deterministic no-LLM test (`tests/coordination/orchestrator/test_real_runtime_parallel_race.py`) forces the racy interleave via `asyncio.gather` + a yielding `execute_step` — RED on the pre-fix code, GREEN after. See ADR-010. - **Anthropic adapters: empty-but-SUCCESSFUL stream terminals become typed classified errors (or the truncation placeholder) instead of an UNKNOWN ValidationError** (`models/adapters/anthropic_oauth.py`, `models/adapters/anthropic.py`, `models/adapters/streaming.py`, `agents/exceptions.py`). Sequel to the stream-failure entry below, which covered failed streams but left empty output synonymous with truncation: an HTTP-200 stream that legitimately produced no content — stop_reason `refusal`, an empty `end_turn`, an empty `model_context_window_exceeded`, or a stream that closed without any terminal — still constructed `HarmonizedResponse(content=None)`, died in the model validator, and was wrapped by the generic handler as a non-retryable `MODEL_API_UNKNOWN_ERROR` with the provider's terminal signal destroyed (this crashed a production Spren daemon on every boot-replay of one parked event). Four changes: (1) all three SSE readers (the OAuth adapter's sync + async accumulators and the shared `AnthropicStreamAccumulator`) capture `message_delta`'s nullable `stop_details` — decoration for error messages, never branched on (the documented contract: branch on `stop_reason`, not `stop_details`); (2) the finish_reason normalization widens to BOTH deterministic-truncation terminals — `max_tokens` AND `model_context_window_exceeded` (live by default on Sonnet 4.5+) normalize to `length`, so an empty context-window-exceeded response takes the existing truncation placeholder instead of being misclassified transient; (3) both adapters' `harmonize_response` generalize the empty branch — zero text, zero tool calls, zero thinking either takes the `length` placeholder or raises `ModelAPIError.from_provider_response` with a new `empty_completion_payload` marker (shared helper in `streaming.py`; deliberately NOT the in-stream `error` key shape, which keeps firing first and separately) carrying only `stop_reason` + `stop_details`; (4) a new status-less arm in `from_provider_response` classifies by stop_reason — `refusal` → new **`REFUSAL`** classification, non-retryable, message carries the refusal fact plus `stop_details` category/explanation when present; empty `end_turn` → new **`EMPTY_COMPLETION`** classification, non-retryable (Anthropic guidance: don't retry empty responses without modification; the suggested action says to send a modified request / continuation prompt); `stop_sequence` / unknown terminals / no terminal at all → `EMPTY_COMPLETION` retryable. Enum choice: `REFUSAL` is its own value rather than a flavor of `EMPTY_COMPLETION` because a content-policy decline and a degenerate empty stream demand different downstream handling, and consumers must not parse message text to tell them apart; both values are additive (`is_critical` membership unchanged; both fall to `get_error_action`'s TERMINAL default — the pre-existing PARTIAL_FAILURE pattern — while the adapter retry ladder reads `is_retryable`). Recorded, not fixed: on the API-key adapter's NON-streaming path `base.py`'s generic handlers re-wrap the typed raise (message survives, classification degrades to UNKNOWN in the returned `ErrorResponse`) — a strict improvement over the ValidationError it replaces; the streaming path (`arun_streaming`) propagates it typed. The OAuth adapter's thinking-only latent gap (thinking set, no text/tool calls, non-length terminal → still a ValidationError) stays recorded-open and unreachable there; the API-key twin already closed it. - **Anthropic adapters: stream failures and empty completions no longer crash harmonization** (`models/adapters/anthropic_oauth.py`, `models/adapters/anthropic.py`, `agents/exceptions.py`). Production failure (Spren daemon, 2026-06-11): an in-stream SSE `error` event (Anthropic delivers stream failures under HTTP 200 — `overloaded_error` ≙ HTTP 529) was silently dropped by the OAuth adapter's SSE accumulator, the empty result harmonized, and `HarmonizedResponse`'s own validator rejected it — surfacing as an opaque `MODEL_API_UNKNOWN_ERROR` ValidationError with the provider's real message destroyed. Three changes restore the contract "every stream outcome maps to either a valid `HarmonizedResponse` or a typed classified `ModelAPIError`": (1) both stream readers (sync + async) capture `type:"error"` events and stop reading (partials are discarded by design — recovery is a new request, and partial `tool_use` must never execute; the discarded-output LENGTH, never its text, is annotated on the raised error); (2) `ModelAPIError.from_provider_response` accepts a plain error dict (the status-less in-stream case) and classifies by the documented error type (`overloaded_error` → SERVICE_UNAVAILABLE/retryable, `rate_limit_error` → RATE_LIMIT/retryable, auth/permission/invalid-request → their classes; unknown types keep UNKNOWN but the real provider message); (3) both Anthropic adapters now put the NORMALIZED token into `ResponseMetadata.finish_reason` (`max_tokens` → `length` — the contract split the schema documents and `openai.py` already implements; the raw token stays on `stop_reason`), and a truncation that produced zero output gets the cross-adapter placeholder content (openai convention) so callers never see `content=None`. Latent gap recorded, not fixed: a thinking-only response (thinking set, no text/tool calls, `end_turn`) would still fail the validator — unreachable while no caller enables extended thinking on these adapters. ### Added +- **Deferred tool loading across the model-adapter layer (provider tool-search / `defer_loading`)** (Framework Session 17; `models/adapters/{anthropic,anthropic_oauth,openai,openai_oauth,openrouter,google}.py`, `models/models.py`). A per-tool `defer_loading: true` flag on a tool dict marks it for on-demand discovery: the provider's tool-search built-in finds it when needed and the definition rides the message *tail*, so the deferred tool's schema stays out of the billed/cached request prefix (`tools → system → messages`) and the prompt cache survives a mid-conversation load. The flag rides the existing `tools` array — **no `arun`/`run` signature change** (a separate `deferred_tools` kwarg would be warn-dropped by OpenAI's closed `valid_openai_params` allowlist; the per-tool flag is read inside the array the adapter already converts). Per-provider translation (DP-006): **Anthropic** (api-key + OAuth) maps the flag onto the Anthropic tool and auto-adds the `tool_search_tool_regex_20251119` server tool; **OpenAI Responses** (api-key + OAuth/Codex) maps it onto the flat function tool and auto-adds the `{"type":"tool_search"}` built-in; the search tool is non-deferred by construction (the API requires ≥1 non-deferred tool) and auto-add is suppressed if the caller supplies their own. Providers WITHOUT the feature handle it explicitly rather than silently: **OpenRouter** STRIPS the flag before its verbatim `tools` forward (a `defer_loading` on the wire would 400) and warns; **Google** warns + falls back to eager (its tool rebuild already drops the flag). **Additive / zero-regression:** with nothing deferred, every adapter's request payload is byte-identical to before — no `defer_loading` key, no search tool, no header change (the Anthropic Tool Search Tool is now GA, so no `anthropic-beta` header is required — confirmed live, incl. on the Claude-Code-impersonating OAuth path). The response side needed NO change: the discovered tool's `tool_use` already surfaces through every harmonizer; the `server_tool_use` / `tool_search_tool_result` round-trip blocks are dropped harmlessly (the API re-expands `tool_reference` from the `tools=` definitions, so the discovery round-trip is coherent without carrying them — verified live, OAuth, search→load→use→answer). `local.py` is out of scope (separate `LocalProviderAdapter` base, tools unsupported). Multi-consumer: any framework agent/workflow with a large tool or MCP catalog (the documented provider answer to tool-token cost + selection-accuracy degradation past ~30–50 tools), MARSYS Cloud / Studio hosted fleets, third-party MCP users, and Spren's `ToolDisclosure` (the cache-clean backend it swaps to). Tests: `tests/models/test_deferred_tool_loading.py` (per-adapter native shape + the identity guarantee + the openrouter-strip / google-warn leak-prevention + the cache-prefix-stable-across-a-discovery-round-trip proof). +- **Agent control directive `escalate_to_user` (scoped, durable)** (Framework Session 18; ADR-013; `coordination/formats/coordination_tools.py`, `validation/response_validator.py`, `formats/context.py`, `execution/step_executor.py`, `agents/agents.py`, `execution/real_runtime.py`, `execution/orchestrator_types.py`, `execution/orchestrator.py`, `formats/base.py`). A *granted* agent emits `escalate_to_user(prompt)` mid-task to durably suspend the run for human input **without a pre-wired topology `User` node** — the dynamic counterpart to the static `ask_user`/`UserNode` path. It is `ask_user` with the gate AXIS swapped from "topology edge to User" to a per-agent grant: a new framework-generic `BaseAgent(..., can_escalate=False)` capability (default OFF — no agent gains it implicitly) gates BOTH the tool-schema offer (`CoordinationContext.can_escalate_user` → `CoordinationToolSchemaBuilder.build_schemas`) and validation (`_validate_escalate_user`), so an ungranted agent is neither offered the tool nor passes its validation. A validated directive translates to a NEW `OrchestratorStepResult(kind="ESCALATE_USER")` (the `StepKind` Literal gains the member) that the orchestrator's `_interpret` routes DIRECTLY into framework 16's durable seam — `enqueue_user_interaction(branch, prompt, resume_agent=branch.current_agent, durable=True)` — with no topology `User` node and `resume_agent` = the emitting agent, so `resume_session(user_response=…)` re-runs that same agent with the response. The durable suspend/resume machinery (FW16) is reused UNCHANGED: an escalation produces the same `pending_user_interaction` snapshot shape and resumes via the same path (a distinct `ESCALATE_USER` step-kind + direct orchestrator route was chosen over reusing the `ask_user` User-node route, which is topology-bound and resumes the node's *successor*; see ADR-013 for the rejected edge-less-`UserNode` alternative). A separate, grant-gated instruction block (`base.py`) teaches the agent when to escalate — deliberately NOT folded into the topology-gated WORKFLOW COMPLETION block (which early-returns on a User-less/End-only topology, the re-auth case). The `ask_user`/`UserNode` static path and every existing coordination tool are unchanged (`can_escalate` defaults off → existing agents behave identically). Multi-consumer: Spren browser re-auth (S62 grants its browsing agent), MARSYS Cloud operator approvals, CI human gates. The general control-directive / flow-control primitive (automatic raisers; a `pause`/`redirect`/`fail` vocabulary) is DEFERRED to a second consumer (ADR-013). +- **`AgentSpec.can_escalate` — the `escalate_to_user` grant serialized through the workflow wire format** (ADR-013 wire-mirror completion / first consumer Spren S62; `agents/serialize.py`). FW18 added `can_escalate` to the live `Agent`/`BaseAgent` constructor but not to `AgentSpec`, the "wire mirror of `Agent`'s constructor surface" — so the grant could not round-trip through a saved workflow definition, leaving every spec-hydrated agent ungrantable. Adds `can_escalate: bool = False` to `AgentSpec`, carried through `agent_to_pydantic` (live→spec) and both `pydantic_to_agents` constructor paths (base + specialized), mirroring `bidirectional_peers` exactly. Backward-compatible (optional, default `False` — an at-rest spec lacking the field hydrates to `False` despite `AgentSpec`'s `extra="forbid"`); the live `Agent.can_escalate` stays canonical (read at dispatch). A fixed-signature specialized factory that does not accept it (e.g. `BrowserAgent.create_safe`) keeps the documented `_accepted_kwargs` drop, same as `bidirectional_peers`. Round-trip tests in `tests/agents/test_serialize.py`. +- **Durable human-in-the-loop suspend/resume** (Framework Session 16; ADR-012; `coordination/execution/orchestrator.py`, `orchestra.py`, `det_nodes.py`, `orchestrator_types.py`, `state/snapshot.py`). Makes the `ask_user`/`UserNode` wait *durable*: a workflow reaching a DURABLE user interaction snapshots to disk and `Orchestra.execute()` returns paused-awaiting-user (`metadata["paused"]=True` **and** `metadata["awaiting_user"]=True`; `WorkflowResult.error="awaiting_user"`, a second pause sentinel beside `"paused"`) WITHOUT spawning an in-memory wait or hitting the 300s timeout. After an arbitrary wait and/or a full process restart, `Orchestra.resume_session(session_id, user_response=…)` reconstructs the suspended branch, injects the response via the existing `resume_branch_with_user_response` seam, and continues dispatch to terminal. The existing SYNC `ask_user` path is unchanged (`durable=False` default; the durable path bypasses `_drive`/`CommunicationMode` entirely). Surface: `enqueue_user_interaction(..., *, durable=False)` (impl + the `DetNodeContext` protocol); `UserNode(..., durable=False)`, also declarable in a workflow definition via a USER node's `metadata["durable"]` (read from the canonical source node by the legacy shim, forward-compatible to the v0.4 generic det-node path); a new `StateSnapshot.pending_user_interaction: Optional[UserInteractionState]` capturing the in-flight durable interaction (held apart from the queued-siblings deque, whose FIFO pop would mis-dispatch it; `Optional`+default keeps pre-ADR-012 snapshots valid under `extra='forbid'`); `resume_session` gains `user_response` (additive, keyword-only, rebased onto FW17's `canonical_topology`/`on_bus_rebuilt` — supplying it with no pending durable interaction, or omitting it with one pending, raises). Multi-consumer: Spren browser re-auth (via the FW18 `escalate_to_user` directive), MARSYS Cloud operator-approval gates, CI human gates, local overnight approvals. ADR-007's on-demand pause/resume is preserved unchanged. v0.3 is single-pending-durable (durable siblings serialize FIFO via the existing queue). +- **`Orchestra.resume_session` resume-ergonomics for cross-process consumers** (Framework Session 17; ADR-011; `coordination/orchestra.py`). Two additive, optional, keyword-only params let a consumer that did NOT `execute()` the run in this process resume a snapshot cleanly. `canonical_topology=` binds the `topology_graph` internally (the snapshot deliberately does not carry topology) via a new private `_build_topology_graph` extracted from `execute()` — the single analyze + legacy-shim + validate path, so the resumed graph is *equivalent* to execute's (digest-matched), not the non-equivalent graph a consumer gets by poking `topology_graph` directly. `on_bus_rebuilt: Callable[[EventBus], None]` is invoked once after the resume EventBus rebuild AND after the topology-bound + digest-match preconditions pass (never on a failed-precondition resume, so a consumer's subscribers never attach to a bus a failing resume discards), letting the consumer re-attach its own `EventBus.subscribe(...)` listeners (e.g. a per-run cost/telemetry adapter) that the unconditional resume bus rebuild would otherwise drop. Both default to today's behavior — every existing `resume_session(session_id)` caller (and the existing pause/resume tests) is unchanged. Multi-consumer: Spren cross-process resume-from-disk with per-run cost tracking, MARSYS Cloud node-migration resume, CI process-B resume. `execute()` is refactored to call `_build_topology_graph` (behavior-preserving — the full pre-existing pause/resume + topology suites stay green). - **Opt-in adapter streaming with a caller delta tap + Anthropic extended thinking** (`models/adapters/streaming.py` new; `anthropic.py`, `openai.py`, `base.py`, `anthropic_oauth.py`, `openai_oauth.py`, `response_models.py`). The API-key async adapters (`AsyncAnthropicAdapter`, `AsyncOpenAIAdapter`) implement `arun_streaming()` behind a per-instance opt-in (`streaming=True` ctor kwarg; class default stays False — existing constructions untouched; the sync twins ignore the kwarg). A transport-agnostic accumulation layer parses the provider SSE grammars and rebuilds the REST response shape, so streamed calls are harmonized by the SAME `harmonize_response` as non-streamed ones — parity by construction, including usage (Anthropic input tokens arrive on `message_start`, output tokens on `message_delta`; both merged). Stream OPENS retry the standard retryable statuses with backoff (no delta has been emitted before a 200, so retries never duplicate observation); once deltas flow, failures are terminal per the stream-failure contract above. Callers observe deltas live via an `on_stream_event` tap (`adapters.streaming.StreamTap`; popped at the `arun` seam like `trace_ctx` so payload builders never see it; a raising tap is logged and disabled, never failing the call). The OAuth twins adopt the tap inside their existing readers (tap-only; their accumulation loops are otherwise unchanged). **Anthropic extended thinking end-to-end**: a positive `thinking_budget` (the `BaseAPIModel` convention, auto-injected per call) adds the `thinking` payload block (temperature dropped — the API rejects sampling params with thinking; budget clamped under `max_tokens` with a warning); harmonization extracts thinking text plus STRUCTURAL blocks (signature included; `redacted_thinking` opaque) onto `reasoning_details` — the existing opaque-provider-blocks carrier Gemini thought signatures ride — and the payload builder re-emits a message's own block types verbatim ahead of text/tool_use (the tool-use round-trip requirement; foreign carrier types are filtered out). The thinking-only latent gap recorded in the Fixed entry above is closed at the harmonize layer (`content=""` — a valid shape — instead of a ValidationError). - **Confined `file_operations` dispatcher in `FileOperationTools.get_tools()`** (`environment/file_operations/core.py`): `get_tools()` now returns a ninth entry, `file_operations`, a single-call dispatcher routing the `operation` argument (`read` / `write` / `edit` / `list` / `info` / `delete`) onto the instance's confined tools, honoring the instance's `FileOperationConfig` (including `enable_delete`; disabled operations return a structured error, they don't raise). Lets a consumer expose ONE file tool name to an agent while keeping every call inside the instance's confinement root. The `content` parameter docstring leads with the "REQUIRED when operation=write/edit" contract so schema-only consumers pass it correctly. - **AG-UI event stream translator** (Framework Session 06; `marsys.coordination.aggui`): a framework-internal adapter that subscribes to `EventBus` and emits AG-UI protocol events as an async iterator, so any UI that speaks AG-UI (SSE UIs, hosted dashboards, third-party clients) can render a live MARSYS run with no per-consumer translation logic. Public exports: `AGGUITranslator` (the EventBus subscriber; constructed inside `Orchestra._wire_event_bus()` so resumed sessions also produce streams), `AGUIEventStream(translator)` (async iterator), `aggui_event_to_sse(event) -> str` (thin wrapper around `ag_ui.encoder.EventEncoder`), `AGGUIConfig` (`enabled: bool = False`, `queue_max_size: int = 10000`), `MarsysRunState` (typed snapshot of branches / barriers / plans / total_steps). Optional dependency: `pip install 'marsys[aggui]'` (pulls `ag-ui-protocol==0.1.18` and `jsonpatch>=1.33`). diff --git a/docs/architecture/framework/decisions/ADR-011-resume-ergonomics.md b/docs/architecture/framework/decisions/ADR-011-resume-ergonomics.md new file mode 100644 index 00000000..192ba7ba --- /dev/null +++ b/docs/architecture/framework/decisions/ADR-011-resume-ergonomics.md @@ -0,0 +1,102 @@ +# ADR-011: Resume Ergonomics for External Cross-Process Consumers + +**Status**: Proposed +**Date**: 2026-06-24 +**Implements**: Framework Session 17 — `docs/implementation/spren/v0.3.0/13-unified-browser-and-pause-resume/framework/17-resume-ergonomics.md` (co-located under the Spren bundle per founder convention) +**Related**: ADR-007 (pause/resume snapshot — the substrate this makes externally usable) + +## Context + +ADR-007 shipped durable on-demand pause/resume (`Orchestra.pause_session` / `resume_session`, cross-process, restart-surviving). But `resume_session` is only cleanly usable by a consumer that `execute()`d the run in the *same* process. A consumer that reconstructs a paused run from disk in a *fresh* process (Spren Session 61 re-materializes the frozen workflow → builds a new `Orchestra` → resumes) hits two gaps: + +1. **Topology must be pre-bound, but it can only be built inside `execute()`.** `resume_session` raises `StateError(RESUME_NO_TOPOLOGY)` unless `self.topology_graph` is already set (`orchestra.py:1439-1445`). The snapshot deliberately does not carry the topology (`:1434-1438`). The only code that builds an *equivalent* `topology_graph` — `analyze` + `_apply_legacy_topology_shim` + `validate`/`validate_workflow` — lives inline in `execute()` (`:1024-1042`) and is `None` until then (`:430`). A consumer *can* poke `orch.topology_graph` / `orch.canonical_topology` directly (the existing tests do, `test_pause_resume.py:563`), but that skips the analyze/shim/validate pipeline → a graph **not equivalent** to execute's, which then mismatches the snapshot's `topology_digest`. + +2. **The resume bus rebuild silently drops the consumer's custom listeners.** `resume_session` rebuilds the EventBus (`self.event_bus = EventBus(); self._wire_event_bus()`, `:1427-1428`), restoring only the standard listener set (StatusManager, TraceCollector, AGGUITranslator). A consumer's own `bus.subscribe(...)` — e.g. a per-run cost/telemetry adapter — is lost, and the rebuild + dispatch happen *inside* `resume_session`, so there is no external seam to re-attach after the new bus exists. + +This is CRITICAL-tier per the framework `CLAUDE.md`: it extracts a shared helper from a TRUNK-CRITICAL method (`Orchestra.execute`) and adds parameters to a TRUNK-CRITICAL method (`Orchestra.resume_session`). The changes are additive (two optional, keyword-only params) + a behavior-preserving refactor, but the policy mandates an ADR before code. + +These are **general** resume-ergonomics gaps, not Spren-specific: any cross-process resume consumer (a hosted control plane resuming on a fresh node after migration/restart; a CI integration where process B resumes what process A snapshotted) needs topology re-binding and custom-listener preservation. + +## Decision + +### 1. Extract `_build_topology_graph` (the single topology-build path) + +Extract `execute()`'s topology-build block (`orchestra.py:1024-1042`) into a private helper, mirroring the framework's existing execute/resume-sharing idiom (`_initialize_per_topology` `:286`, `_wire_event_bus` `:318` — both `self`-mutating private helpers called by both paths). + +```python +def _build_topology_graph(self, canonical, execution_config) -> None: + """The single topology-build path (extracted from execute()): analyze + + legacy shim + validate. Sets BOTH self.canonical_topology and + self.topology_graph. Called by execute() and resume_session().""" + self.canonical_topology = canonical + canonical.metadata = canonical.metadata or {} + canonical.metadata.setdefault("auto_inject_user", False) + self.topology_graph = self.topology_analyzer.analyze(canonical) + self.topology_graph.metadata["execution_config"] = execution_config + self._apply_legacy_topology_shim(self.topology_graph, canonical) + self.topology_graph.validate() + self.topology_graph.validate_workflow() +``` + +- **Sets both** `self.canonical_topology` and `self.topology_graph` — the resume digest check (`_compute_topology_digest`, `:1617`) and `_initialize_per_topology` (`:310`) both read `self.canonical_topology`, so setting only `topology_graph` would pass the guard but then mis-digest or build rules against a stale canonical. +- **`execute()` keeps `:1013-1023`** — `_ensure_topology(topology)`, execution-config resolution + `context["execution_config"]`, and the `auto_inject_user` read from execute's `context` — that prep is caller-local and has no `context` on the resume path. `execute()` keeps its explicit `canonical.metadata["auto_inject_user"] = context.get(...)` set *before* calling the helper (so its behavior is unchanged); the helper's `setdefault(False)` is a no-op for execute and the safe default for resume. The trace-update block (`:1044-1053`, reads execute-only `session_id`/`trace_collector`) also stays in `execute()`. + +### 2. Two additive, optional, keyword-only params on `resume_session` + +```python +async def resume_session( + self, session_id: str, *, + canonical_topology=None, + on_bus_rebuilt: "Callable[[EventBus], None] | None" = None, +) -> OrchestraResult: ... +``` + +- **`canonical_topology`** — when supplied, bind via `_build_topology_graph(canonical_topology, execution_config)` *before* the `RESUME_NO_TOPOLOGY` guard (`:1439`) and the digest check (`:1448`). A cross-process consumer need not pre-set `topology_graph`/`canonical_topology`. +- **`on_bus_rebuilt`** — when supplied, invoked once as `on_bus_rebuilt(self.event_bus)` **after** the guard and the digest check pass (after `:1453`, before `_initialize_per_topology` `:1459` / dispatch `:1509`), so the consumer re-attaches its own subscribers to the rebuilt bus. + +The bus rebuild stays at `:1427-1428` (the listener-rebuild test, `test_pause_resume.py:567-577`, depends on rebuild-before-digest ordering). Both params default to `None` → today's behavior for every existing caller. + +### 3. `on_bus_rebuilt` fires only on a resume that proceeds + +The callback fires **after** the precondition checks (topology bound + digest match). On a `RESUME_NO_TOPOLOGY` or `IncompatibleSnapshotError` abort it does **not** fire — so a consumer's subscribers never attach to a bus a failing resume discards, and a raising callback cannot preempt the real precondition error. (Coordinate the keyword-only signature with the future ADR-012 `user_response` param — distinct additive concerns.) + +## Rationale + +- **Why extract a helper, not inline the build into resume.** AC-2 (resume produces the graph *equivalent* to execute's) is only honest with a single source. Duplicating analyze+shim+validate into resume is the add-parallel failure mode and would drift the moment the (v0.4-bound) shim changes. Extraction matches the established `_initialize_per_topology`/`_wire_event_bus` idiom — a third `_build_*`/`_initialize_*` helper a reader expects. +- **Why two kwargs, not a `bind_topology()` method or an options object.** The two concerns are orthogonal (topology bind vs listener restore). The listener-restore concern has no home *outside* `resume_session` (the rebuild lives there), so a separate `bind_topology()` would split one resume-ergonomics concern across two surfaces and still need a kwarg on `resume_session`. An options dataclass is premature abstraction for two optional args. +- **Why `on_bus_rebuilt` fires post-precondition.** Validate before side effects: running consumer code on a resume that's about to raise `IncompatibleSnapshotError` is the wrong order, and it lets a callback error mask the real precondition error. On a failed resume there are no run events and the bus is discarded, so the callback firing there buys nothing. +- **Why the helper defaults `auto_inject_user`.** It is an *input* to `analyze()` (`analyzer.py`) that changes the analyzed graph's node/edge structure (a legacy User-node injection) but is **not** part of the digest. A resume whose supplied `canonical_topology` carried a different `auto_inject_user` than the original run would silently build a divergent graph. The helper defaults it `False` (the new-style case); legacy auto-inject topologies are documented NOT cross-process-resumable in v0.3, and the digest-ignores-metadata gap is RISK-LOGGED for a future snapshot-format PR. + +## Alternatives considered + +- **Public `bind_topology(canonical)` method** — rejected: splits one resume-ergonomics concern across two API surfaces (you still need a kwarg on `resume_session` for `on_bus_rebuilt`), and is heavier than the private-helper idiom the codebase already uses. +- **Standing persistent-subscriber registry on `Orchestra`** — rejected: a heavier, stateful mechanism for what a one-shot post-rebuild callback solves. +- **Fire `on_bus_rebuilt` at the bus rebuild (`:1428`), symmetric with `_wire_event_bus`** — rejected: it runs consumer code on doomed (digest-mismatch / no-topology) resumes and lets a raising callback preempt the precondition error; firing post-precondition matches the callback's purpose (prepare subscribers for the upcoming dispatch). +- **Serialize the topology into the snapshot** (so no rebind needed) — deferred: a snapshot-format change beyond this slice's additive scope (ADR-007 already flags it as a future PR). + +## Consequences + +### Backward compatibility +- Fully back-compat. Both params are optional + keyword-only and default to today's behavior; `resume_session(session_id)` is unchanged, including the existing tests that set `orch.topology_graph` directly. The `_build_topology_graph` extraction is behavior-preserving for `execute()` (verified by the existing execute/topology suite staying green before the resume call is added). + +### Multi-consumer +- **Spren (S61)** — reconstruct + resume a paused run from disk with per-run cost tracking intact (the immediate driver). +- **MARSYS Cloud** — resume on a fresh node after migration/restart, with telemetry subscribers preserved. +- **CI / local consumers** — process B resuming a run process A snapshotted. +- No `from spren` import; no "if Spren" path (SP-018). + +### Known limitations +- Legacy `auto_inject_user` topologies are not cross-process-resumable in v0.3 (the digest does not cover the flag). RISK-LOGGED for a future snapshot-format PR. +- A consumer supplying a `canonical_topology` whose digest differs from the snapshot's gets a clean `IncompatibleSnapshotError` after the bind (not a silent wrong-topology run). + +## Completion — the bus rebuild must re-point reused emitters (2026-06-25) + +`on_bus_rebuilt`'s motivating use case — re-attaching a per-run cost adapter so post-resume LLM spend is billed — did not actually work as first shipped. Spren's S61 **live** test (real OAuth model, real pause→resume) found it: `resume_session` rebuilds `self.event_bus` and hands consumers the new bus via `on_bus_rebuilt`, and `_wire_event_bus` re-creates the listener set (TraceCollector / StatusManager / AGGUITranslator) on it — **but the REUSED `step_executor`** (which emits `LLMCallEvent`, the event cost is computed from) **and `_user_node_handler` still held the prior bus.** So the resumed dispatch's LLM events published on the stale bus, and a consumer re-attached via `on_bus_rebuilt` (subscribed to the new bus) received nothing. FW17's own resume test missed this because it used a **stub agent that makes no LLM call** — `on_bus_rebuilt` was only ever exercised against orchestrator-level events (`BranchCompletedEvent`), never a real `LLMCallEvent`. + +Fix: `_wire_event_bus` now also re-points the reused publishers (`step_executor.event_bus`, `_user_node_handler.event_bus`) to `self.event_bus`, guarded (they are created *after* this call in `__init__`, so the guards no-op there and bind the fresh bus at construction; on resume they exist and get re-pointed). With this, a resumed run's `LLMCallEvent`s reach the rebuilt bus and `on_bus_rebuilt` delivers its full contract. Regression: the framework pause/resume suite (34 tests) stays green; Spren's `test_pause_resume_live.py` asserts `total_cost_usd` accrues across a real resume. + +## Approval + +This ADR requires framework-team approval before merge. Approval is recorded here by the framework lead, OR by an explicit approval message in the PR thread. + +- [ ] Framework lead approval: _pending_ diff --git a/docs/architecture/framework/decisions/ADR-012-durable-hitl-suspend.md b/docs/architecture/framework/decisions/ADR-012-durable-hitl-suspend.md new file mode 100644 index 00000000..50f571fa --- /dev/null +++ b/docs/architecture/framework/decisions/ADR-012-durable-hitl-suspend.md @@ -0,0 +1,119 @@ +# ADR-012: Durable Human-in-the-Loop Suspend/Resume + +**Status**: Proposed +**Date**: 2026-06-26 +**Implements**: Framework Session 16 — `docs/implementation/spren/v0.3.0/13-unified-browser-and-pause-resume/framework/16-durable-hitl-suspend.md` (co-located under the Spren bundle per founder convention) +**Related**: ADR-007 (pause/resume snapshot — the substrate this extends), ADR-011 (resume ergonomics — `resume_session`'s keyword-only params this rebases onto), ADR-013 (the `escalate_to_user` directive — the immediate v0.3 consumer, lands after this) + +## Context + +ADR-007 shipped durable **on-demand** pause/resume (`Orchestra.pause_session` / `resume_session`, cross-process, restart-surviving). It never composed pause with the human-in-the-loop `ask_user` wait. Today a `UserNode`/`ask_user` interaction is **synchronous and in-memory**: `Orchestrator.enqueue_user_interaction` spawns an `asyncio.create_task(_drive())` (orchestrator.py:422-445) that awaits `UserNodeHandler.handle_user_node`, which blocks on an `asyncio.Future` with a default 300s timeout. Three gaps follow: + +1. **The in-flight interaction is not durably captured.** `enqueue_user_interaction` appends to `self._user_interactions` only when an interaction is *already* in-flight (orchestrator.py:409-411) — i.e. only the queued siblings. The in-flight interaction's `(prompt, resume_agent, delivery_target)` lives only in the `_drive` closure + the WAITING branch. `snapshot()` serializes the deque (orchestrator.py:1340) → the in-flight prompt/resume_agent is lost; `restore_from()` rebuilds `_resume_user_responses = None` (orchestrator.py:1377) and never re-drives it. +2. **The dispatch loop blocks on the in-memory wait.** With no runnable branches and a pending interaction, the loop blocks on `await self._resume_user_responses.get()` (orchestrator.py:273-282). While blocked it cannot re-check the pause flag, so `pause_session` on a `UserNode`-parked run hangs until the human answers or the 300s timeout fires (`quiesce()` awaits `_loop_exited_event`, orchestrator.py:328-333). +3. **No resume-with-a-response surface.** `resume_session` (ADR-007/011) continues a paused run but has no way to inject a human's answer into the suspended branch. + +The need — "a workflow pauses pending an out-of-band human action that may exceed 300s and survive a restart" — is exactly "make `ask_user` durable." This is a focused EXTENSION of ADR-007, not a new mechanism: the suspended branch is already an ordinary `WAITING` branch that `snapshot()` deep-copies (orchestrator.py:1332), and `resume_branch_with_user_response` (orchestrator.py:462-494) is a **synchronous** response-injection seam with no dependency on the `_drive` closure or the `asyncio.Future` — so the in-flight wait need not stay an in-memory coroutine. + +This is CRITICAL/TRUNK-CRITICAL: it makes a non-additive behavior change to `Orchestrator._dispatch_loop_inner` (the user-wait park) and `Orchestrator.enqueue_user_interaction` (a durable arm), widens the `DetNodeContext` protocol, and adds an additive parameter to `Orchestra.resume_session`. Per the framework `CLAUDE.md`, an ADR is mandatory before code. + +These are **general** durable-HITL gaps, not Spren-specific (multi-consumer below). + +## Decision + +### 1. A `durable` arm on `enqueue_user_interaction` + a single scalar capture + +`Orchestrator.enqueue_user_interaction` gains a keyword-only `durable: bool = False`. The non-durable arm (the in-memory `_drive`/Future/300s path) is **unchanged**. The durable arm: marks the branch `WAITING`, records the interaction in a single scalar `self.pending_user_interaction`, sets `self._user_interaction_inflight = True` (so a second interaction still queues as a sibling), and **spawns no `_drive`** (no Future, no timeout). + +The in-flight interaction is captured as **one scalar**, NOT folded into the `_user_interactions` deque. Folding is unsafe: `resume_branch_with_user_response` pops every deque entry as a *queued sibling* and re-dispatches it (orchestrator.py:487-494), so a folded in-flight item would be mis-dispatched on resume — which is exactly why the in-flight interaction is already excluded from the deque (orchestrator.py:409-411). The scalar follows the existing live/wire split: + +- live `Orchestrator.pending_user_interaction: Optional[tuple]` — the `(suspended_branch_id, prompt, resume_agent, delivery_target)` shape, matching the deque's items; +- `OrchestratorState.pending_user_interaction: Optional[tuple] = None`; +- `StateSnapshot.pending_user_interaction: Optional[UserInteractionState] = None` — the typed wire shape (reuses the existing `UserInteractionState`, snapshot.py:84-97). `Optional` + default `None` keeps pre-ADR-012 snapshots valid under `extra='forbid'`. + +Mapping reuses the existing `_user_interaction_to_state` (orchestra.py:1804) forward + a one-line reverse, exactly as the deque maps — so `execution/orchestrator.py` gains no import of the snapshot wire model. The name `pending_user_interaction` is deliberately distinct from the existing bool `user_interaction_inflight`. + +`resume_agent` is persisted (not re-derived on resume): `UserNode._resume_agent_for` needs the live topology successors + `branch.last_invoked_agent` (det_nodes.py:145-152), and `last_invoked_agent` is written only on the agent-continuation hop (real_runtime.py:140), never on the det-node hop. Persisting it is deterministic and means a topology edit during a human-timescale wait cannot silently re-route the resume (consistent with the version/digest lock). + +### 2. Dispatch-loop snapshot-and-exit at the durable boundary + +In `_dispatch_loop_inner`, when there are no runnable branches and no in-flight ticks, a **durable** pending interaction returns a new `_build_awaiting_user_result()` instead of blocking on `_resume_user_responses.get()`: + +```python +if not in_flight: + if self.pending_user_interaction is not None: + return self._build_awaiting_user_result() # durable: snapshot-and-exit + if self._user_interaction_inflight or self._user_interactions: + if self._resume_user_responses is not None: # SYNC path: unchanged + ...await self._resume_user_responses.get()... + break +``` + +The durable-exit keys off the **scalar**, so `restore_from` re-hydrating the scalar is what makes the resumed loop behave correctly. `_build_awaiting_user_result()` (beside `_build_paused_result`, orchestrator.py:1297) returns `WorkflowResult(error="awaiting_user")` — a second pause sentinel parallel to `"paused"`. + +### 3. `Orchestra.execute()` / `resume_session()` self-pause + injection + +- **`_snapshot_and_write(session_id, orchestrator)`** is extracted from `pause_session`'s existing write block (orchestra.py:1409-1411 — `_build_state_snapshot` → `model_dump_json` → `storage_backend.write`) and shared by both call sites. +- **`execute()`** (and **`resume_session()`**, since a resumed run can itself re-pause awaiting-user) treat `workflow.error in ("paused", "awaiting_user")` as paused; on `"awaiting_user"` they `await self._snapshot_and_write(...)` **inside the try, before the `finally` pops `_active_orchestrators`**, and set the public `metadata["awaiting_user"] = True` (mirroring how `metadata["paused"]` is derived). `FinalResponseEvent` stays suppressed for both pause flavors. +- **`resume_session`** gains `user_response`, additive to ADR-011's signature → the combined **keyword-only-optional** surface: + +```python +async def resume_session( + self, session_id: str, *, + canonical_topology=None, on_bus_rebuilt=None, user_response=None, +) -> OrchestraResult: ... +``` + + After loading the snapshot it validates (AC-5): `user_response` provided but `snapshot.pending_user_interaction is None` → `ValueError`; a pending durable interaction but `user_response is None` → `ValueError` (you cannot resume a human-wait without the answer). When both are present it injects **before** `resume()` via the existing public seam `orchestrator.resume_branch_with_user_response(suspended_branch_id, user_response, resume_agent)`. That seam's tail (its existing in-flight-consumption logic, orchestrator.py:487-494) also clears `self.pending_user_interaction = None` — a no-op on the SYNC path. No new injection method; no orchestra.py poking of orchestrator internals beyond the existing tightly-coupled lifecycle surface. + +### 4. The durable trigger — `UserNode(durable=)` flag, expressible in code AND in a workflow definition + +The durable signal is a `durable: bool` flag (extend-and-unify), NOT a `CommunicationMode` value (the durable path bypasses `_drive`/`handle_user_node` entirely, so it never enters the `CommunicationMode`-consuming legacy handler) and NOT a new node kind (a sibling `DurableUserNode` would duplicate `UserNode`'s `_resume_agent_for`/`on_*` for one bool): + +- `DetNodeContext.enqueue_user_interaction` (orchestrator_types.py:264) and its impl gain `*, durable: bool = False` (additive protocol widening); +- `UserNode.__init__` gains `durable: bool = False`; `on_single_invoke`/`on_dispatch` pass `durable=self.durable`; +- **workflow-definition expressibility** (founder decision 2026-06-26): a USER node carrying `durable` in the topology spec materializes a `UserNode(durable=True)`. In v0.3 the USER node rides the legacy shim (orchestra.py:638-645); the shim reads `durable` from the canonical source node (it receives the canonical topology, which retains node metadata) and constructs `UserNode(durable=...)`. Written forward-compatibly so the v0.4 generic-materialization path (analyzer.py:211-213) inherits it when the USER carve-out is removed. + +### 5. `communication/` is NOT touched + +The durable arm spawns no `_drive`, so `handle_user_node` / `CommunicationMode` are never on the durable path. The SYNC `ask_user` path keeps them unchanged (back-compat). Routing "durable" through `CommunicationMode` would add a mode to a subsystem the durable path provably never enters. + +## Rationale + +- **Why extend, not a parallel suspend mechanism.** The frame's disconfirmer — a constraint forcing the in-flight wait to stay an in-memory coroutine — is absent: `resume_branch_with_user_response` is synchronous and reconstructs the resume branch from `(branch_id, response, resume_agent)` alone (orchestrator.py:462-494). The suspended branch is already first-class durable state. So a durable interaction reuses snapshot/restore + the injection seam verbatim; a parallel mechanism would be the add-parallel failure mode. +- **Why a scalar, not the deque.** The deque's FIFO pop treats entries as queued siblings (orchestrator.py:487-494); the in-flight interaction is the *current* one, deliberately excluded from the deque (:409-411). A scalar slot is the honest shape. +- **Why a second `error` sentinel, not a new `WorkflowResult` field.** Pause already travels as a sentinel string on `.error`, and both translation sites are string checks (orchestra.py:1163, :1570). Reusing the channel adds one branch per site; a new field would thread a parallel signal through two layers. +- **Why a `durable` flag on `UserNode`, expressible in the spec.** Extend-and-unify on the existing user-interaction node. Making it spec-expressible (vs constructor-only) is the founder's call (full declarative capability now); the v0.3 cost is the shim read, written to survive the v0.4 carve-out removal. + +## Alternatives considered + +- **Fold the in-flight interaction into `_user_interactions`** — rejected: the FIFO pop (orchestrator.py:487-494) would re-dispatch it as a sibling on resume. +- **Hack `pause_session` to self-pause from inside the loop** — rejected: `pause_session` calls `quiesce()` + the already-terminal race guard, both wrong for a loop that exited *itself*. Extract `_snapshot_and_write` and call it directly. +- **Keep the in-memory Future + a longer timeout** — rejected: does not survive a restart and still blocks the loop; the whole point is durability across process death. +- **A `CommunicationMode.ASYNC_*` durable mode** — rejected: consumed only by the legacy `handle_user_node` the durable path never enters. +- **A sibling `DurableUserNode` kind** — rejected: duplicates `UserNode` for one bool (anti-pattern #2). +- **Constructor-only durable trigger (no spec expressibility)** — considered; overridden by founder decision to ship the full declarative capability. + +## Consequences + +### Backward compatibility +Fully back-compat. `durable` defaults `False` (SYNC `ask_user` unchanged — AC-6); `resume_session`'s new param is keyword-only optional and defaults to today's behavior (ADR-007 on-demand resume unchanged — AC-5); the new snapshot field is `Optional` + default `None` (old snapshots still validate under `extra='forbid'`). `_build_topology_graph` and the FW17 resume machinery are reused unchanged. + +### Multi-consumer +- **Spren (the immediate v0.3 consumer)**: FW18's `escalate_to_user` directive (ADR-013) routes to `enqueue_user_interaction(durable=True)` with no topology User node — the S62 browser re-auth flow. The durable `UserNode` (code + spec) is the general pre-wired-durable-user-step capability, not the S62 path. +- **MARSYS Cloud**: operator-approval gates on managed long-runs that survive node restarts. +- **CI integrations**: a human gate spanning multiple CI jobs (process A suspends; process B resumes). +- **Framework local users**: an overnight approval that survives a reboot. +- No `from spren` import; no "if Spren" path (SP-018). + +### Known limitations +- **Version-lock during a human-timescale wait.** The snapshot is locked to the exact `framework_version` (ADR-007). A framework upgrade *during* a long re-auth wait makes the run unresumable (`IncompatibleSnapshotError`) — more likely than for on-demand pause. Surfaced as an honest failure, not silent corruption. +- **Single-pending durable (v0.3).** One `resume_session(user_response)` answers the one in-flight durable interaction; durable siblings serialize FIFO. Sufficient for the real consumer (one re-auth per run). Multi-pending-durable ergonomics are deferred. +- **At-least-once re-run (ADR-007).** The suspended branch is `WAITING` (not mid-tool-call) at the durable boundary, so the resume's re-run surface is the resume_agent's first tick — no worse than ADR-007. +- **`auto_inject_user` / metadata not in the digest** (inherited from ADR-011) — a durable USER node's `durable` flag rides the workflow definition (re-analyzed on resume), and the in-flight datum is authoritative from the snapshot scalar, so resume does not re-derive it; a topology edit changing `durable` during a wait is governed by the same digest limitation ADR-011 already documents. + +## Approval + +This ADR requires framework-team approval before merge. Approval is recorded here by the framework lead, OR by an explicit approval message in the PR thread. + +- [x] Framework lead (founder) approved the design + full scope to proceed on 2026-06-26 (the implementer's Phase-A synthesis gate; founder chose option 2 — the workflow-definition durable trigger included). Formal merge sign-off occurs at central submodule-PR integration. diff --git a/docs/architecture/framework/decisions/ADR-013-agent-control-directive.md b/docs/architecture/framework/decisions/ADR-013-agent-control-directive.md new file mode 100644 index 00000000..e4ae7511 --- /dev/null +++ b/docs/architecture/framework/decisions/ADR-013-agent-control-directive.md @@ -0,0 +1,115 @@ +# ADR-013: Agent Control Directive — `escalate_to_user` + +**Status**: Proposed +**Date**: 2026-06-26 +**Implements**: Framework Session 18 — `docs/implementation/spren/v0.3.0/13-unified-browser-and-pause-resume/framework/18-agent-control-directive.md` (co-located under the Spren bundle per founder convention) +**Related**: ADR-003 (topology-driven routing — the model this bounded-departs from), ADR-007 (pause/resume snapshot substrate), ADR-011 (resume ergonomics — `resume_session`'s keyword-only surface), ADR-012 (durable HITL — the durable `enqueue_user_interaction`/`resume_session(user_response)` seam this directive routes into) + +## Context + +Today the only agent→human path is `ask_user`, which the framework routes to a topology `User` det-node: validation maps it to `ActionType.ASK_USER` (gated on `has_edge_to_usernode`, `response_validator.py:227`), `_translate` returns `StepResult(kind="SINGLE_INVOKE", next_agent="User", value=question)` (`real_runtime.py:258-264`), and `_interpret`→`_handle_single_invoke`→`get_det_node("User")`→`UserNode.on_single_invoke` reaches the suspend seam (`orchestrator.py:744-747`, `det_nodes.py:159-168`). The capability is offered to the agent only when the topology wired a `User` edge (the schema gate `coordination_tools.py:122`, fed by `CoordinationContext.can_ask_user`). + +That is a *pre-wired* human step. Spren v0.3 Bundle 13 — Session 62 (browser re-auth) — needs the *dynamic* case: a borrowed-browser workflow whose agent *perceives mid-run* that it has been logged out and must escalate for a human to re-authenticate, then resume. Such a workflow has no `User` node, and injecting one per-workflow is topology surgery (it also changes the graph's static readability and routes resume to the node's *successor*, not the asking agent). The dynamic escalation is **general**, not Spren-specific (multi-consumer below). + +ADR-012 shipped the load-bearing half: a durable suspend seam — `enqueue_user_interaction(branch, prompt, resume_agent, *, durable=True)` records the in-flight interaction in `pending_user_interaction`, the dispatch loop snapshots-and-exits (`orchestrator.py:281-285`), and `resume_session(user_response=…)` spawns a fresh branch at `resume_agent` with the response as input (`orchestrator.py:489-540`, `orchestra.py:1609-1614`). Crucially, that seam is **agent- and topology-agnostic**: `resume_branch_with_user_response` reconstructs the resume from `(branch_id, response, resume_agent)` alone, and the snapshot persists `resume_agent`. So an escalation needs no `User` node — only a way to *reach* the durable seam from a dynamically-emitted agent directive. + +This is CRITICAL/TRUNK-CRITICAL: it adds a coordination directive (new `ActionType`, a `COORDINATION_TOOL_NAMES` member, a validator arm, a tool schema), a new `OrchestratorStepResult` kind with an orchestrator dispatch arm, an additive `CoordinationContext` field, and a framework-generic agent capability. Per the framework `CLAUDE.md`, an ADR is mandatory before code. + +## Decision + +### 1. A new coordination directive `escalate_to_user(prompt)` + +A granted agent emits `escalate_to_user(prompt)` as a native coordination tool call. It travels the same emit→validate→translate→route pipeline `ask_user` does, with the gate axis changed (see §2) and a distinct route (see §3): + +- **Membership**: `escalate_to_user ∈ COORDINATION_TOOL_NAMES` (`coordination_tools.py:23`) — so `is_coordination_tool` classifies it on the coordination path (`step_executor.py:1033-1040`) and it is never handed to `ToolExecutor` as a regular tool. +- **Schema**: a new `_build_escalate_user_schema()` (a `prompt: str` param, mirroring `_build_ask_user_schema`), appended by `build_schemas` when the agent is granted (§2). +- **Validation**: `ActionType.ESCALATE_USER` + a `validate_coordination_action` dispatch arm + `_validate_escalate_user` — `_validate_ask_user` with the gate axis swapped (§2), validating a non-empty `prompt`, returning `parsed_response={"prompt": prompt}`. + +### 2. Availability is SCOPED — a per-agent grant, not topology, not ambient + +`escalate_to_user` is offered and accepted only for an agent explicitly granted it. The grant is a framework-generic agent capability `can_escalate: bool` (default **`False`**), threaded as `CoordinationContext.can_escalate_user` (`context.py`), derived in `_build_coordination_context` (`step_executor.py:857-915`, where the agent instance is in scope at the call site `:352`), and consumed at BOTH gates: + +- **schema offer** — `build_schemas(..., can_escalate_user=…)` appends the schema only when granted; +- **validation** — `_validate_escalate_user` rejects (`is_valid=False`, permission category) when `getattr(agent, "can_escalate", False)` is false. + +So escalate is `ask_user` with the gate AXIS swapped from "topology edge to `User`" to "agent is granted `can_escalate`". The topology-edge axis (`has_edge_to_usernode`, `can_ask_user`) is untouched. + +**Why scoped, not universal (founder decision 2026-06-26).** Every other coordination capability in the framework is *granted*, never ambient — universal availability would make escalate the sole exception, and it would let any agent in any workflow durably suspend an unattended/scheduled run waiting for a human who may never look. Scoped is also asymmetric in cost: scoped→universal is a one-line change (`can_escalate_user=True` unconditionally in `_build_coordination_context`); universal→scoped is a breaking change once agents rely on ambient availability. The S62 consumer grants the capability to its browsing agent. + +### 3. A new `ESCALATE_USER` step-kind, routed by the orchestrator directly to the durable seam + +`_translate` maps a validated `ESCALATE_USER` action to a new `OrchestratorStepResult(kind="ESCALATE_USER", value=prompt)` (the prompt sourced from `validation.parsed_response`, not an undefined local). `"ESCALATE_USER"` is added to the `StepKind` Literal (`orchestrator_types.py:49-55`). A new `_interpret` arm (`orchestrator.py:703-731`) routes it directly into the durable seam: + +```python +if step.kind == "ESCALATE_USER": + self.enqueue_user_interaction( + br, step.value, resume_agent=br.current_agent, durable=True + ) + return +``` + +`br.current_agent` IS the emitting agent at the escalate boundary, so resume targets that agent — stated explicitly, not derived. No topology `User` node; no parallel suspend path; the FW16 durable machinery (capture → snapshot-and-exit → `resume_session(user_response)`) is reused verbatim. An escalation produces the SAME `pending_user_interaction` shape FW16 consumes, so snapshot/restore/resume need no change. + +### 4. `durable=True` is passed directly (decoupled from `UserNode` metadata) + +For the `ask_user` durable path (ADR-012 §4), durability rides `UserNode.durable`, read from the spec node's `metadata["durable"]`. Escalate has no node, so the `_interpret` arm passes the literal `durable=True`. Escalate is inherently durable: the dynamic-escalation use case (a human goes away and returns, possibly across a restart) defeats the in-memory SYNC wait. There is no non-durable escalate variant in v0.3. + +### 5. The instruction surface is gated on the grant, separate from the workflow-completion block + +A granted agent's system prompt gets an `escalate_to_user` behavioral-contract block (when/how to escalate, that the run pauses durably and resumes it with the reply), gated on `coord.can_escalate_user` and appended in `build_complete_system_prompt` (`base.py:73-77`). It is **not** folded into `_build_workflow_completion_instructions` (`base.py:160-190`), which early-returns unless `can_terminate_workflow or can_ask_user` — folding would hide the escalate contract in exactly the S62 case (a User-less, End-edge browsing workflow). Instruction surface ↔ tool surface stay matched on the same grant, the invariant `base.py:148-155` documents. + +## Rationale + +- **Why a new step-kind, not reusing the `ask_user` route.** Escalate and `ask_user` are two distinct concepts — dynamic/node-less/resume-self vs pre-wired/topology-bound/resume-successor. `ask_user`'s route requires a registered `User` det-node and resumes the node's *successor* (`det_nodes.py:150-157`), both wrong for escalate. A distinct, orchestrator-interpreted directive is the day-one-correct shape; the parallelism is two concepts each with one clean path, which the Fit discipline's brake explicitly endorses (do not force two concepts into one mechanism). `_interpret` extending with a sixth arm is a legitimate use of the `StepKind` dispatch axis — distinct from the `NodeKind`/det-node registry axis, which is the one that is extension-closed on dispatch. +- **Why route through the orchestrator, not synthesize a `User` node.** The rejected alternative (a registered-but-edge-less `UserNode`) would inject a phantom `User` node into workflows that declare none and would require widening the TRUNK-CRITICAL topology reachability validator (`graph.py:1375-1382` errors on any non-End node unreachable from Start) and co-opting `_resume_agent_for`'s fallback to get resume-self implicitly. The new-kind route needs zero validator change and states resume-self explicitly. +- **Why reuse FW16's seam.** The durable suspend/resume is already agent/topology-agnostic; escalate is simply a second *producer* of the same durable interaction. A parallel suspend mechanism would be the add-parallel failure mode. + +## Alternatives considered + +- **Reuse the `ask_user` route via a registered, edge-less `UserNode`** — rejected: phantom node in node-less workflows; requires widening the topology reachability validator; resume-self becomes an implicit fallback rather than an explicit contract. +- **Universal (always-on) availability** — rejected by founder: makes escalate the only ambient coordination capability; risks unattended runs stalling on spurious escalation; universal→scoped would be a breaking change. (Scoped→universal stays a one-line option.) +- **Per-workflow `User`-node injection** — rejected: topology surgery; the very thing FW18 exists to avoid. +- **A Spren-only escalate tool** — rejected: a Spren tool can't reach the orchestrator's suspend seam, and it would violate SP-018 (no framework→Spren coupling). The capability is framework-generic; the consumer grants it. +- **Folding the escalate instruction into the workflow-completion block** — rejected: that block early-returns on User-less/End-edge topologies, hiding the contract in the primary consumer's case. + +## Consequences + +### Backward compatibility +Fully back-compat. `can_escalate` defaults `False` — no agent gains the tool by default; a default-constructed agent is neither offered `escalate_to_user` nor passes its validation, and its full prior coordination-action set is otherwise unchanged. The `ask_user`/`User`-node static path (schema gate + validation + routing + resume-successor) is untouched. The new `StepKind` member and `CoordinationContext` field are additive. The FW16 durable/resume machinery and the topology validators are reused unchanged. + +### Multi-consumer +- **Spren (the immediate v0.3 consumer)**: S62 grants `can_escalate` to its browsing agent; on a detected logged-out wall the agent escalates → durable suspend → human re-auths in the main browser → `resume_session(user_response)` → the agent re-perceives a fresh authed page and continues. No `from spren` import; no "if Spren" branch (SP-018). +- **MARSYS Cloud**: an agent hitting an out-of-band operator approval it couldn't pre-wire. +- **CI integrations**: a dynamic human gate spanning jobs (process A suspends; process B resumes). +- **Framework local users**: any agent granted the capability that needs mid-task human input without a graph `User` step. + +### Known limitations +- **At-least-once re-run (ADR-007/012).** The branch is `WAITING` at the escalate boundary; the resume's re-run surface is the resumed agent's first tick — no worse than ADR-012. +- **Single-pending durable (v0.3, inherited from ADR-012).** One `resume_session(user_response)` answers the one in-flight durable interaction; durable siblings serialize FIFO. Sufficient for one re-auth per run. +- **Version-lock during a human-timescale wait (inherited from ADR-007/012).** A framework upgrade during a long escalation wait makes the run unresumable — an honest failure, not silent corruption. +- **Agent must perceive the need to escalate.** Escalation is the agent's judgment; an agent that fails to recognize it should escalate won't. This is the consumer's coaching problem (S62's browse-guidance), not a framework gap; the deferred automatic-condition raiser (below) could add a backstop later. +- **Spurious escalation stalls an unattended run.** Mitigated by scoping (only granted agents can escalate) and recoverability (a stalled run is discarded/resumed by a human; no state corruption). + +### The topology-driven-routing departure (ADR-003) +`escalate_to_user` suspends without a graph `User` node — a bounded, explicit departure from ADR-003: ONE interpreted directive, routed by the orchestrator, not arbitrary imperative flow. The run's static graph no longer fully predicts its human-interaction points for escalate-granted agents; this is the deliberate price of dynamic escalation, owned here and kept bounded by the deferral below. + +## Deferred (the general control-directive / flow-control primitive) + +Founder-requested direction (2026-06-24), **NOT built here** — recorded so the direction is on record: +- **Generalize the raiser**: an automatic condition hook (per-step/on-signal) that raises a directive — the "watchdog" (e.g. cost > X, a tool returns Y). +- **Generalize the action set**: beyond `escalate_to_user` — `pause` (durable, no input), `redirect → agent`, `fail` — a uniform control-directive vocabulary the orchestrator interprets. +- **Attachment model**: where a hook attaches (agent/node/workflow) and how it's configured. +- **Open design questions**: condition source (agent vs automatic); the action vocabulary; how dynamic directives reconcile with topology-driven routing (ADR-003); precedence when multiple hooks fire. +- **Build trigger**: a SECOND concrete consumer (anti-pattern #4/#11). `escalate_to_user` is consumer #1; it ships as a single arm, not a framework for hooks no one calls. No `pause`/`redirect`/`fail` `ActionType` values or extra `can_*` flags are pre-added now. + +## Addendum (2026-06-26) — `AgentSpec` wire-mirror completion (Spren S62) + +FW18 added `can_escalate` to the live `Agent`/`BaseAgent` constructor (the canonical runtime grant, read at dispatch via `getattr(agent, "can_escalate", …)`) but not to `AgentSpec`, the serialized "wire mirror of `Agent`'s constructor surface". So the grant could not round-trip through a saved workflow definition — an agent hydrated from `AgentSpec` (every workflow agent) could never be granted escalate. The first consumer (Spren S62, browser re-auth) needs exactly that: a browser-workflow agent granted escalate. + +S62 completes the mirror: add `can_escalate: bool = False` to `AgentSpec`, carried through `agent_to_pydantic` (live→spec) and both `pydantic_to_agents` constructor paths (base + specialized) — mirroring `bidirectional_peers` exactly. Both layers are canonical by role, not redundant: the live `Agent` attribute is what dispatch reads (a pure-Python `Agent(can_escalate=True)` needs no spec), and the `AgentSpec` field is its serialization (round-trips through `workflow.json` and survives a cold-resume's re-read of the frozen definition). Backward-compatible: an at-rest spec lacking the field hydrates to `False`. The framework stays Spren-agnostic (SP-018) — the consumer sets the grant; for a fixed-signature specialized factory whose constructor does not accept it (e.g. `BrowserAgent.create_safe`), `_accepted_kwargs` drops it, the same documented property `bidirectional_peers` has. + +## Approval + +This ADR requires framework-team approval before merge. Approval is recorded here by the framework lead, OR by an explicit approval message in the PR thread. + +- [x] Framework lead (founder) approved the design + full scope to proceed on 2026-06-26 (the implementer's Phase-A synthesis gate; founder chose the corrected improver-A shape + SCOPED availability). Formal merge sign-off occurs at central submodule-PR integration. diff --git a/docs/architecture/framework/design-principles.md b/docs/architecture/framework/design-principles.md index 557ca737..94fbc3d5 100644 --- a/docs/architecture/framework/design-principles.md +++ b/docs/architecture/framework/design-principles.md @@ -221,6 +221,8 @@ Explanation: Direct API calls bypass adapter harmonization and error handling. **Exceptions**: None. New providers must be added via `ProviderAdapterFactory` and adapter classes. +**Capability — deferred tool loading (Framework Session 17)**: the canonical example of the DP-006 value. A single normalized input — a per-tool `defer_loading: true` flag on the tool dict (it rides the existing `tools` array, no `arun`/`run` signature change) — is translated per-provider inside each adapter: Anthropic maps it onto the Anthropic tool + auto-adds the `tool_search_tool_regex_20251119` server tool; OpenAI Responses maps it onto the flat function tool + auto-adds the `tool_search` built-in; OpenRouter strips it (no feature, would 400 on the wire) + warns; Google warns + falls back to eager. No provider dialect (the `tool_search`/`defer_loading` shapes, the discovery response blocks) leaks above the adapter boundary; the caller marks "this tool is deferred" once and each adapter does the right thing. Additive + default-off (nothing deferred ⇒ byte-identical request payload per adapter). See CHANGELOG `[Unreleased]` and `tests/models/test_deferred_tool_loading.py`. + --- ## DP-007: Format Pluggability diff --git a/src/marsys/agents/agents.py b/src/marsys/agents/agents.py index c61643eb..a9104df1 100644 --- a/src/marsys/agents/agents.py +++ b/src/marsys/agents/agents.py @@ -145,6 +145,7 @@ def __init__( memory_retention: str = "session", # New parameter memory_storage_path: Optional[str] = None, # New parameter plan_config: Optional[Union[PlanningConfig, Dict, bool]] = None, # Planning configuration + can_escalate: bool = False, # ADR-013: grant for the escalate_to_user directive ) -> None: """ Initializes the BaseAgent. @@ -177,6 +178,12 @@ def __init__( self.model = model self.goal = goal self.instruction = instruction + # ADR-013: per-agent grant for the escalate_to_user control directive. + # Default off — a granted agent may durably suspend the run for human + # input WITHOUT a topology User node. Gates the directive's schema offer + # (can_escalate_user) and its validation. Framework-generic; consumers + # grant it (SP-018). + self.can_escalate = can_escalate # Store planning config for later initialization self._planning_config = PlanningConfig.from_value(plan_config) @@ -1646,6 +1653,7 @@ async def run_step(self, request: Any, context: Dict[str, Any]) -> Dict[str, Any can_ask_user=coordination_context.can_ask_user, is_conversation_branch=getattr(coordination_context, 'is_conversation_branch', False), output_schema=self._compiled_output_schema if hasattr(self, '_compiled_output_schema') else None, + can_escalate_user=getattr(coordination_context, 'can_escalate_user', False), ) else: self._coordination_tool_schemas = [] @@ -2760,6 +2768,7 @@ def __init__( memory_retention: str = "session", memory_storage_path: Optional[str] = None, plan_config: Optional[Union[PlanningConfig, Dict, bool]] = None, + can_escalate: bool = False, ) -> None: """ Initializes the Agent. @@ -2809,6 +2818,7 @@ def __init__( memory_retention=memory_retention, # Pass memory retention memory_storage_path=memory_storage_path, # Pass storage path plan_config=plan_config, # Pass planning configuration + can_escalate=can_escalate, # ADR-013: forward the escalate grant ) self._model_config = model_config self._compaction_model_config = compaction_model_config diff --git a/src/marsys/agents/exceptions.py b/src/marsys/agents/exceptions.py index 2f4e7d34..fd134598 100644 --- a/src/marsys/agents/exceptions.py +++ b/src/marsys/agents/exceptions.py @@ -633,6 +633,55 @@ def __init__( ) +# Transport-layer exception class names → the transient classification they map to. +# Matched by NAME across the raised exception's MRO so ONE table spans httpx, +# httpcore, aiohttp, and the stdlib without importing any of them (httpx is not a +# declared dependency of this package). A transport failure carries no HTTP status +# and no provider error body, so it reaches ``from_provider_response`` with +# ``status_code=None`` and no ``error`` dict — without this it keeps the UNKNOWN, +# non-retryable default, which inverts the truth for the most common transient +# failure on a network-flaky host (DNS blip, dropped wifi, reset connection). +# Names below are the stable public exception classes of those libraries; the +# curated set deliberately EXCLUDES our-side/config faults that share the transport +# tree (httpx ``LocalProtocolError``/``UnsupportedProtocol``, ``DecodingError``, +# ``TooManyRedirects``) — those are not transient and must not be retried. +_TIMEOUT_EXC_NAMES = frozenset({ + "TimeoutException", # httpx / httpcore base for Connect/Read/Write/PoolTimeout + "TimeoutError", # stdlib + asyncio.TimeoutError alias; aiohttp Server/Socket timeouts subclass it +}) +_NETWORK_EXC_NAMES = frozenset({ + "NetworkError", # httpx / httpcore base for Connect/Read/Write/CloseError + "ConnectError", # httpx / httpcore connect failure (DNS included) + "ConnectionError", # stdlib base: ConnectionReset/Aborted/RefusedError + "RemoteProtocolError", # server disconnected / malformed response mid-stream — transient. + # NOTE: match this leaf, NOT the shared base ``ProtocolError`` — + # ``LocalProtocolError`` (our-side bad request framing) also derives + # from it and is NOT transient; it must keep the non-retryable default. + "ProxyError", # httpx proxy hop failed + "ClientConnectionError", # aiohttp base for connector/OS/disconnect errors + "ClientConnectorError", # aiohttp connect failure (DNS included) + "ClientOSError", # aiohttp socket-level error + "ServerDisconnectedError", # aiohttp server dropped the connection + "ServerConnectionError", # aiohttp server connection lost + "ClientPayloadError", # aiohttp connection broken mid-payload +}) + + +def _classify_transport_exception(exception: BaseException) -> Optional[str]: + """Classify a transport-layer exception (no HTTP status, no error body) as a + transient ``TIMEOUT`` or ``NETWORK_ERROR``, or ``None`` if it isn't a recognized + transport failure. Walks the exception's MRO by class NAME (see the name tables) + so it spans httpx / httpcore / aiohttp / stdlib with no import of any of them. + Timeout wins over network when both names appear (aiohttp's timeout types + subclass its connection types) — 'timed out' is the more specific signal.""" + names = {klass.__name__ for klass in type(exception).__mro__} + if names & _TIMEOUT_EXC_NAMES: + return APIErrorClassification.TIMEOUT.value + if names & _NETWORK_EXC_NAMES: + return APIErrorClassification.NETWORK_ERROR.value + return None + + class ModelAPIError(ModelError): """ Enhanced API error with provider-specific error classification. @@ -691,6 +740,7 @@ def __init__( "google": "Enable billing or upgrade from free tier at https://console.cloud.google.com", "openrouter": "Add credits at https://openrouter.ai/credits", "xai": "Check credits at https://console.x.ai/billing", + "bedrock": "Check your AWS account limits and Bedrock model access in the AWS console", "openai-oauth": "Upgrade to ChatGPT Plus/Pro at https://chatgpt.com/upgrade", "anthropic-oauth": "Check your Claude Max subscription at https://claude.ai/settings" } @@ -811,7 +861,10 @@ def from_provider_response( classification = APIErrorClassification.SERVICE_UNAVAILABLE.value is_retryable = True - elif provider == "anthropic": + # Bedrock serves the Messages API and returns the same error + # envelope, so it classifies identically to first-party Anthropic — + # sharing the branch keeps one behaviour for one wire contract. + elif provider in ("anthropic", "bedrock"): error_data = raw_response.get("error", {}) if raw_response else {} if error_data: message = error_data.get("message", message) @@ -1029,9 +1082,11 @@ def from_provider_response( # ValidationError with the provider's terminal signal destroyed). # Classification branches on stop_reason — the documented contract; # stop_details is NULLABLE decoration, appended to the message when - # present and never keyed on. max_tokens/model_context_window_exceeded - # never arrive here: harmonization routes them to the truncation - # placeholder. + # present and never keyed on. Two terminals never arrive here, because + # harmonization represents them instead of raising: + # max_tokens/model_context_window_exceeded (→ truncation placeholder) + # and end_turn (→ a silent turn, content="": the model finished and + # chose to say nothing, which is a success, not a fault). stop_reason = raw_response.get("stop_reason") details = raw_response.get("stop_details") details = details if isinstance(details, dict) else {} @@ -1049,17 +1104,6 @@ def from_provider_response( "The provider declined to answer this request. Modify or " "rephrase it; retrying unmodified will be refused again." ) - elif stop_reason == "end_turn": - # Anthropic's documented guidance: don't retry empty responses - # without modification — the model already decided it was done. - classification = APIErrorClassification.EMPTY_COMPLETION.value - is_retryable = False - message = "Anthropic returned an empty response (stop_reason 'end_turn', no content)" - suggested_action = ( - "Do not retry unmodified — the model decided it was done. " - "Send a modified request, e.g. a continuation prompt asking " - "it to produce the response." - ) else: # stop_sequence, never-seen stop reasons, or NO terminal at all # (stream closed before message_delta): transient — retry. @@ -1077,6 +1121,17 @@ def from_provider_response( f"(stop_reason '{stop_reason}', no content)" ) + # Transport-layer failure: a connect/DNS/timeout/reset error raised by the + # HTTP client before any HTTP status or provider error body exists. It falls + # through every status-code and SSE branch above and would otherwise keep the + # UNKNOWN, non-retryable default — wrong for what is a transient, self-healing + # fault. Only when we're still UNKNOWN with no status code do we classify by + # the raised exception's type, so this never overrides a real provider verdict. + if exception is not None and status_code is None and classification == APIErrorClassification.UNKNOWN.value: + transport_classification = _classify_transport_exception(exception) + if transport_classification is not None: + classification = transport_classification + # Payload-too-large override: some providers return 400 with payload # hints instead of 413. This runs unconditionally so it can override # earlier INVALID_REQUEST-style classifications from 400 status codes. diff --git a/src/marsys/agents/memory_strategies.py b/src/marsys/agents/memory_strategies.py index 0ca0cf5a..5a9c9ea2 100644 --- a/src/marsys/agents/memory_strategies.py +++ b/src/marsys/agents/memory_strategies.py @@ -615,6 +615,7 @@ async def reduce( PROVIDER_PAYLOAD_LIMITS = { "anthropic": 32_000_000, "anthropic-oauth": 32_000_000, + "bedrock": 32_000_000, # same Messages API body limit as first-party Anthropic "openai": 25_000_000, "openai-oauth": 25_000_000, "google": 100_000_000, diff --git a/src/marsys/agents/serialize.py b/src/marsys/agents/serialize.py index 1b74fbf3..e9273edc 100644 --- a/src/marsys/agents/serialize.py +++ b/src/marsys/agents/serialize.py @@ -226,6 +226,9 @@ class AgentSpec(BaseModel): max_tokens: Optional[int] = 10000 allowed_peers: List[str] = Field(default_factory=list) bidirectional_peers: bool = False + # ADR-013: the escalate_to_user grant, mirrored from Agent.can_escalate so it + # round-trips through a serialized workflow (the live Agent stays canonical). + can_escalate: bool = False is_convergence_point: Optional[bool] = None memory_retention: MemoryRetention = "session" memory_storage_path: Optional[str] = None @@ -322,6 +325,7 @@ def agent_to_pydantic(agent: Any) -> AgentSpec: max_tokens=agent.max_tokens, allowed_peers=sorted(agent._allowed_peers_init), bidirectional_peers=agent._bidirectional_peers, + can_escalate=agent.can_escalate, is_convergence_point=agent._is_convergence_point, memory_retention=agent._memory_retention, memory_storage_path=agent._memory_storage_path, @@ -433,6 +437,7 @@ async def pydantic_to_agents( max_tokens=agent_spec.max_tokens, allowed_peers=list(agent_spec.allowed_peers), bidirectional_peers=agent_spec.bidirectional_peers, + can_escalate=agent_spec.can_escalate, input_schema=agent_spec.input_schema, output_schema=agent_spec.output_schema, memory_retention=agent_spec.memory_retention, @@ -484,6 +489,7 @@ async def pydantic_to_agents( max_tokens=agent_spec.max_tokens, allowed_peers=list(agent_spec.allowed_peers), bidirectional_peers=agent_spec.bidirectional_peers, + can_escalate=agent_spec.can_escalate, input_schema=agent_spec.input_schema, output_schema=agent_spec.output_schema, memory_retention=agent_spec.memory_retention, diff --git a/src/marsys/coordination/config.py b/src/marsys/coordination/config.py index 6cfa64c3..57d3bc99 100644 --- a/src/marsys/coordination/config.py +++ b/src/marsys/coordination/config.py @@ -383,6 +383,11 @@ class ErrorHandlingConfig: "base_delay": 1.0, "insufficient_quota_action": "notify", }, + "bedrock": { + "max_retries": 3, + "base_delay": 1.0, + "insufficient_quota_action": "raise", + }, "openrouter": { "max_retries": 2, "base_delay": 1.0, diff --git a/src/marsys/coordination/execution/det_nodes.py b/src/marsys/coordination/execution/det_nodes.py index 4f3c4720..818178d2 100644 --- a/src/marsys/coordination/execution/det_nodes.py +++ b/src/marsys/coordination/execution/det_nodes.py @@ -138,9 +138,14 @@ class UserNode(DeterministicNode): RESERVED_NAME = "User" - def __init__(self, name: str = "User", handler: Any = None): + def __init__(self, name: str = "User", handler: Any = None, durable: bool = False): self.name = name self.handler = handler # UserNodeHandler bound at workflow construction + # ADR-012: when True the interaction suspends the run DURABLY (snapshot- + # and-exit, resumable via resume_session(user_response)) instead of the + # in-memory SYNC wait. Set from the topology spec by the legacy shim + # (and, post-v0.4, the generic det-node materialization). + self.durable = durable def _resume_agent_for(self, ctx, branch) -> str: """Pick the resume agent: first non-self successor, else fall back to @@ -152,14 +157,19 @@ def _resume_agent_for(self, ctx, branch) -> str: return branch.last_invoked_agent or branch.current_agent def on_single_invoke(self, ctx, branch, value): - if self.handler is None: + # A durable interaction bypasses the SYNC handler entirely (no _drive), + # so it needs no bound handler; only the SYNC path requires one. + if not self.durable and self.handler is None: ctx.fail(branch, f"UserNode {self.name!r} has no handler bound") return resume_agent = self._resume_agent_for(ctx, branch) - ctx.enqueue_user_interaction(branch, prompt=value, resume_agent=resume_agent) + ctx.enqueue_user_interaction( + branch, prompt=value, resume_agent=resume_agent, durable=self.durable + ) def on_dispatch(self, ctx, fork, request): - if self.handler is None: + # Durable bypasses the SYNC handler (see on_single_invoke). + if not self.durable and self.handler is None: ctx.fail(fork.resolver_branch_obj, f"UserNode {self.name!r} has no handler bound") return # For the parallel-fork case, spawn a placeholder branch at User and @@ -169,7 +179,9 @@ def on_dispatch(self, ctx, fork, request): agent=self.name, input=request, delivery_target=fork.id, ) resume_agent = self._resume_agent_for(ctx, placeholder) - ctx.enqueue_user_interaction(placeholder, prompt=request, resume_agent=resume_agent) + ctx.enqueue_user_interaction( + placeholder, prompt=request, resume_agent=resume_agent, durable=self.durable + ) # --- Single source of truth ------------------------------------------------- diff --git a/src/marsys/coordination/execution/orchestrator.py b/src/marsys/coordination/execution/orchestrator.py index c5fb0267..13d748ce 100644 --- a/src/marsys/coordination/execution/orchestrator.py +++ b/src/marsys/coordination/execution/orchestrator.py @@ -113,12 +113,19 @@ def __init__( self._completed_emitted: set[str] = set() # User-node interaction queue (FIFO single-pending). Each item: - # (suspended_branch_id, prompt, resume_agent, delivery_target). The - # orchestrator dispatches one at a time; siblings wait. Resumed via + # (suspended_branch_id, prompt, resume_agent, delivery_target, durable). + # The orchestrator dispatches one at a time; siblings wait. Resumed via # the _resume_user_responses async-queue. self._user_interactions: collections.deque = collections.deque() self._user_interaction_inflight: bool = False self._resume_user_responses: Optional[Any] = None # asyncio.Queue lazily created + # The single in-flight DURABLE user interaction (ADR-012): the 5-tuple + # (suspended_branch_id, prompt, resume_agent, delivery_target, durable), + # or None. Held apart from the deque (a deque entry is a queued *sibling* + # the FIFO + # pop re-dispatches). Non-None ⇒ the dispatch loop snapshots-and-exits at + # the next user-wait boundary instead of blocking on an in-memory queue. + self.pending_user_interaction: Optional[tuple] = None # Pause/resume primitive (ADR-007). Both events are lazy-init'd # in _dispatch_loop so they bind to the loop that's actually @@ -271,6 +278,11 @@ async def _dispatch_loop_inner(self) -> WorkflowResult: continue if not in_flight: + if self.pending_user_interaction is not None: + # Durable user wait (ADR-012): snapshot-and-exit instead of + # blocking on an in-memory queue. execute()/resume_session + # write the snapshot and surface the awaiting-user result. + return self._build_awaiting_user_result() if self._user_interaction_inflight or self._user_interactions: # An interaction is still pending; wait for it to land. if self._resume_user_responses is not None: @@ -395,22 +407,37 @@ def spawn_branch_at( return br def enqueue_user_interaction( - self, branch: Branch, prompt: Any, resume_agent: str + self, branch: Branch, prompt: Any, resume_agent: str, *, durable: bool = False ) -> None: """Suspend `branch` (mark WAITING), enqueue or dispatch the interaction. FIFO discipline: only one interaction is dispatched at - a time; the rest wait in self._user_interactions.""" + a time; the rest wait in self._user_interactions. + + durable=True (ADR-012): record the in-flight interaction in + self.pending_user_interaction (so snapshot() captures it) and spawn NO + in-memory `_drive` — the dispatch loop snapshots-and-exits at the next + user-wait boundary and resume_session(user_response) resumes it. The + SYNC (durable=False) path is unchanged.""" import asyncio branch.status = "WAITING" branch.waiting_on = "user_interaction" - item = (branch.id, prompt, resume_agent, branch.delivery_target) + item = (branch.id, prompt, resume_agent, branch.delivery_target, durable) if self._user_interaction_inflight: self._user_interactions.append(item) return self._user_interaction_inflight = True + + if durable: + # Durable (ADR-012): capture the in-flight interaction for the + # snapshot; the dispatch loop snapshots-and-exits at its user-wait + # boundary. No in-memory future, no 300s timeout, no resume queue — + # resume_session(user_response) drives the resume. + self.pending_user_interaction = item + return + # Lazily create the resume queue inside an event loop. if self._resume_user_responses is None: try: @@ -471,6 +498,13 @@ def resume_branch_with_user_response( if suspended is not None and suspended.status != "TERMINATED": suspended.status = "TERMINATED" self._completed_emitted.add(suspended.id) + # The suspended branch left via the user interaction, not a barrier + # delivery — drop it from every barrier candidacy (re-queueing each + # for a fire-check) via the same helper _deliver/_fail_to use, so a + # terminated branch never lingers as a phantom pending candidate that + # would keep its barrier from firing. (Latent SYNC-seam gap surfaced + # by ADR-012's first orchestrator-level UserNode→resume→terminal run.) + self._unregister(suspended, keep=set()) spawned = self._spawn( agent=resume_agent, @@ -484,12 +518,24 @@ def resume_branch_with_user_response( target.candidates.add(spawned.id) spawned.candidate_of.add(delivery_target) + # ADR-012: the durable in-flight interaction (if any) has been consumed + # (its suspended branch was just resumed above). Clear the scalar BEFORE + # re-dispatching the next queued sibling, which may itself be durable and + # re-arm the scalar. No-op on the SYNC path (already None). + self.pending_user_interaction = None if self._user_interactions: - next_branch_id, next_prompt, next_resume, _ = self._user_interactions.popleft() + next_branch_id, next_prompt, next_resume, _next_target, next_durable = ( + self._user_interactions.popleft() + ) next_branch = self.branches.get(next_branch_id) if next_branch is not None: self._user_interaction_inflight = False - self.enqueue_user_interaction(next_branch, next_prompt, next_resume) + # Preserve the sibling's durability — a queued durable interaction + # must stay durable when it goes in-flight (ADR-012 FIFO), not + # silently revert to the SYNC path. + self.enqueue_user_interaction( + next_branch, next_prompt, next_resume, durable=next_durable + ) else: self._user_interaction_inflight = False @@ -682,6 +728,16 @@ def _interpret(self, br: Branch, step: StepResult) -> None: self._fail_to(br, step.error or "unspecified") return + if step.kind == "ESCALATE_USER": + # ADR-013: a granted agent's dynamic escalate-to-user. Route directly + # into framework 16's durable suspend seam — no topology User node. + # resume_agent = the emitting agent (br.current_agent), so resume + # re-runs that same agent with the user's response as its input. + self.enqueue_user_interaction( + br, step.value, resume_agent=br.current_agent, durable=True + ) + return + raise ValueError(f"Unknown step kind: {step.kind}") # ══════════════════════════════════════════════════════════════════ @@ -1311,6 +1367,21 @@ def _build_paused_result(self) -> WorkflowResult: barriers=self.barriers, ) + def _build_awaiting_user_result(self) -> WorkflowResult: + """Result returned when the dispatch loop hits a DURABLE user + interaction (ADR-012) — a second pause sentinel parallel to + `_build_paused_result`'s "paused". `Orchestra.execute()` / + `resume_session()` recognize `error == "awaiting_user"`, write the + snapshot, and set metadata["awaiting_user"]=True — distinct from a + plain on-demand pause. The run is suspended, not terminal.""" + return WorkflowResult( + success=False, + final_response=None, + error="awaiting_user", + branches=self.branches, + barriers=self.barriers, + ) + # ══════════════════════════════════════════════════════════════════ # Snapshot / restore (ADR-007) # ══════════════════════════════════════════════════════════════════ @@ -1339,6 +1410,7 @@ def snapshot(self) -> "OrchestratorState": completed_emitted=set(self._completed_emitted), user_interactions=[copy.deepcopy(item) for item in self._user_interactions], user_interaction_inflight=self._user_interaction_inflight, + pending_user_interaction=copy.deepcopy(self.pending_user_interaction), max_steps=self.max_steps, ) @@ -1370,6 +1442,7 @@ def restore_from(self, state: "OrchestratorState") -> None: copy.deepcopy(item) for item in state.user_interactions ) self._user_interaction_inflight = state.user_interaction_inflight + self.pending_user_interaction = copy.deepcopy(state.pending_user_interaction) self.max_steps = state.max_steps # _resume_user_responses (asyncio.Queue) is intentionally rebuilt # fresh on resume — it lives only inside an active loop. Pending @@ -1400,8 +1473,10 @@ class OrchestratorState: root_barrier_id: Optional[str] workflow_error: Optional[str] completed_emitted: set[str] - user_interactions: list # deque content; tuples of (bid, prompt, agent, target) + user_interactions: list # deque content; (bid, prompt, agent, target, durable) user_interaction_inflight: bool + # ADR-012: the single in-flight DURABLE interaction (a 5-tuple) or None. + pending_user_interaction: Optional[tuple] = None max_steps: int = MAX_STEPS_DEFAULT diff --git a/src/marsys/coordination/execution/orchestrator_types.py b/src/marsys/coordination/execution/orchestrator_types.py index 3272f9b7..094d9724 100644 --- a/src/marsys/coordination/execution/orchestrator_types.py +++ b/src/marsys/coordination/execution/orchestrator_types.py @@ -51,6 +51,9 @@ "SINGLE_INVOKE", "PARALLEL_INVOKE", "FINAL_RESPONSE", + # ADR-013: a granted agent's dynamic escalate-to-user directive. The + # orchestrator routes it into framework 16's durable suspend seam. + "ESCALATE_USER", "FAIL", ] @@ -262,12 +265,17 @@ def spawn_branch_at( """Spawn a fresh branch at an agent (used by Start).""" def enqueue_user_interaction( - self, branch: Branch, prompt: Any, resume_agent: str + self, branch: Branch, prompt: Any, resume_agent: str, *, durable: bool = False ) -> None: - """Mark the calling branch as waiting on user input, schedule async I/O - through the configured UserNodeHandler, and (when the user responds) - deliver the response into the orchestrator's resume queue. Used by - UserNode.on_single_invoke.""" + """Mark the calling branch as waiting on user input. + + durable=False (default): schedule async I/O through the configured + UserNodeHandler and (when the user responds) deliver the response into + the orchestrator's resume queue — the in-memory SYNC path. + durable=True (ADR-012): record the in-flight interaction so snapshot() + captures it and spawn NO in-memory wait; the dispatch loop snapshots- + and-exits at the next user-wait boundary, and resume_session(user_response) + resumes it. Used by UserNode.on_single_invoke / on_dispatch.""" def resume_branch_with_user_response( self, suspended_branch_id: str, response: Any, resume_agent: str diff --git a/src/marsys/coordination/execution/real_runtime.py b/src/marsys/coordination/execution/real_runtime.py index 2d6bd6ec..b0492403 100644 --- a/src/marsys/coordination/execution/real_runtime.py +++ b/src/marsys/coordination/execution/real_runtime.py @@ -263,6 +263,16 @@ async def _translate( value=question, ) + if action == ActionType.ESCALATE_USER: + # ADR-013: dynamic escalate-to-user. A distinct step kind (NOT the + # ASK_USER User-node route) the orchestrator routes into framework + # 16's durable suspend. Prompt is sourced from the validator output. + prompt = (validation.parsed_response or {}).get("prompt") + return OrchestratorStepResult( + kind="ESCALATE_USER", + value=prompt, + ) + if action == ActionType.TERMINAL_ERROR: return OrchestratorStepResult( kind="FAIL", diff --git a/src/marsys/coordination/execution/step_executor.py b/src/marsys/coordination/execution/step_executor.py index 1d77ea8c..9e9cf6eb 100644 --- a/src/marsys/coordination/execution/step_executor.py +++ b/src/marsys/coordination/execution/step_executor.py @@ -352,7 +352,8 @@ async def execute_step( coordination_context = self._build_coordination_context( agent_name, topology_graph, - branch=branch + branch=branch, + can_escalate=getattr(agent, "can_escalate", False), ) # Add coordination context and system prompt builder to run_context @@ -859,6 +860,7 @@ def _build_coordination_context( agent_name: str, topology_graph: Optional['TopologyGraph'], branch: Optional['ExecutionBranch'] = None, + can_escalate: bool = False, ) -> CoordinationContext: """ Build CoordinationContext from topology. @@ -911,6 +913,7 @@ def _build_coordination_context( next_agents=next_agents, can_terminate_workflow=can_terminate, can_ask_user=can_ask_user, + can_escalate_user=can_escalate, is_conversation_branch=is_conversation, ) diff --git a/src/marsys/coordination/formats/base.py b/src/marsys/coordination/formats/base.py index d3829399..817e9e37 100644 --- a/src/marsys/coordination/formats/base.py +++ b/src/marsys/coordination/formats/base.py @@ -76,6 +76,14 @@ def build_complete_system_prompt(self, context: SystemPromptContext) -> str: if completion_instructions: parts.append(completion_instructions) + # 3b. Escalation instruction (ADR-013). Gated on the per-agent + # can_escalate grant, SEPARATE from the completion block above (which is + # empty unless the agent can terminate_workflow or ask_user) so a granted + # agent on a User-less, End-edge topology still sees this contract. + escalate_instructions = self._build_escalate_instructions(context) + if escalate_instructions: + parts.append(escalate_instructions) + # 4. Peer agent instructions peer_instructions = self._build_peer_agent_instructions(context) if peer_instructions: @@ -189,6 +197,32 @@ def _build_workflow_completion_instructions( lines.append("--- END WORKFLOW COMPLETION ---") return "\n".join(lines) + def _build_escalate_instructions(self, context: SystemPromptContext) -> str: + """Build the conditional instruction for the `escalate_to_user` directive + (ADR-013), gated on the per-agent `can_escalate` grant. + + Kept SEPARATE from ``_build_workflow_completion_instructions`` because + that block early-returns unless the agent can terminate_workflow or + ask_user (topology-gated) — a granted agent on a User-less, End-edge + topology (the re-auth browsing case) would otherwise never see this + contract. Instruction surface matches tool surface: both gate on + ``can_escalate_user``.""" + coord = context.coordination + if not getattr(coord, "can_escalate_user", False): + return "" + + return ( + "\n\n--- ESCALATION ---\n" + "**Escalating to the human.** When you hit something only a human " + "can resolve mid-task — re-authentication, an approval, or a " + "decision or input you cannot obtain yourself — call the " + "`escalate_to_user` tool with a clear `prompt` describing what you " + "need. The run pauses durably (it survives a restart) and resumes " + "you with the human's reply. Prefer continuing on your own; " + "escalate only when genuinely blocked on a human.\n" + "--- END ESCALATION ---" + ) + def _strip_schema_hints(self, text: str) -> str: """Remove lines that re-explain the output format from agent instructions.""" # With coordination tools, fewer patterns to strip since routing is via tool calls diff --git a/src/marsys/coordination/formats/context.py b/src/marsys/coordination/formats/context.py index 66e0dd1b..ffcb9987 100644 --- a/src/marsys/coordination/formats/context.py +++ b/src/marsys/coordination/formats/context.py @@ -45,6 +45,10 @@ class CoordinationContext: next_agents: List[str] = field(default_factory=list) can_terminate_workflow: bool = False can_ask_user: bool = False + # ADR-013: gate for the escalate_to_user directive. Unlike can_ask_user + # (topology edge to a User node), this is set from the per-agent can_escalate + # grant in _build_coordination_context — escalate is granted, not topology-wired. + can_escalate_user: bool = False is_conversation_branch: bool = False diff --git a/src/marsys/coordination/formats/coordination_tools.py b/src/marsys/coordination/formats/coordination_tools.py index c6c7a6b2..7876479e 100644 --- a/src/marsys/coordination/formats/coordination_tools.py +++ b/src/marsys/coordination/formats/coordination_tools.py @@ -24,6 +24,7 @@ "invoke_agent", "terminate_workflow", "ask_user", + "escalate_to_user", "end_conversation", # REMOVE-IN-V0.4: legacy alias for "terminate_workflow"; kept so agents # emitting the old name still validate. See DEPRECATIONS.md. @@ -83,6 +84,7 @@ def build_schemas( can_ask_user: bool = False, is_conversation_branch: bool = False, output_schema: Optional[Dict[str, Any]] = None, + can_escalate_user: bool = False, ) -> List[Dict[str, Any]]: """ Build coordination tool schemas for an agent. @@ -95,6 +97,8 @@ def build_schemas( edge to User det-node) is_conversation_branch: Whether this agent is in a conversation branch output_schema: Optional output schema to merge into terminate_workflow + can_escalate_user: Whether this agent can call escalate_to_user (gated + on the per-agent can_escalate grant, NOT topology — ADR-013) Returns: List of OpenAI-format tool definition dicts @@ -124,6 +128,11 @@ def build_schemas( CoordinationToolSchemaBuilder._build_ask_user_schema() ) + if can_escalate_user: + schemas.append( + CoordinationToolSchemaBuilder._build_escalate_user_schema() + ) + if is_conversation_branch: schemas.append( CoordinationToolSchemaBuilder._build_end_conversation_schema() @@ -239,6 +248,37 @@ def _build_ask_user_schema() -> Dict[str, Any]: }, } + @staticmethod + def _build_escalate_user_schema() -> Dict[str, Any]: + return { + "type": "function", + "function": { + "name": "escalate_to_user", + "description": ( + "Escalate to the human and pause the run until they respond. " + "Use this when you hit something only a human can resolve " + "mid-task — re-authentication, an approval, or a decision or " + "input you cannot obtain yourself. The run suspends durably " + "(it survives a restart) and resumes you with the human's " + "reply. Prefer continuing on your own; escalate only when " + "genuinely blocked on a human." + ), + "parameters": { + "type": "object", + "properties": { + "prompt": { + "type": "string", + "description": ( + "What you need from the human, stated clearly " + "(e.g. 'Please re-authenticate to example.com')." + ), + }, + }, + "required": ["prompt"], + }, + }, + } + @staticmethod def _build_end_conversation_schema() -> Dict[str, Any]: return { diff --git a/src/marsys/coordination/orchestra.py b/src/marsys/coordination/orchestra.py index d484e5aa..7783ac2e 100644 --- a/src/marsys/coordination/orchestra.py +++ b/src/marsys/coordination/orchestra.py @@ -15,7 +15,7 @@ from dataclasses import dataclass, field from datetime import datetime, timedelta, timezone from pathlib import Path -from typing import Any, Dict, List, Optional, Set, Tuple, TYPE_CHECKING +from typing import Any, Callable, Dict, List, Optional, Set, Tuple, TYPE_CHECKING from .. import __version__ as _MARSYS_VERSION from ..agents.registry import AgentRegistry @@ -283,6 +283,31 @@ async def _run_retention_sweep(self) -> None: except Exception as exc: # pragma: no cover logger.warning("Orchestra: retention sweeper failed: %s", exc) + def _build_topology_graph( + self, + canonical: 'Topology', + execution_config: 'ExecutionConfig', + ) -> None: + """The single topology-build path (extracted from execute()): analyze + + legacy shim + validate. Sets BOTH self.canonical_topology and + self.topology_graph, so the resume digest check and per-topology setup + (both of which read self.canonical_topology) see a coherent pair. + + ``canonical`` is an already-coerced Topology (execute() runs + _ensure_topology first; a cross-process resume consumer supplies one). + ``auto_inject_user`` metadata defaults to False when absent — execute() + sets it explicitly from its run context before calling this, so the + default only applies to the new-style resume path. + """ + self.canonical_topology = canonical + canonical.metadata = canonical.metadata or {} + canonical.metadata.setdefault("auto_inject_user", False) + self.topology_graph = self.topology_analyzer.analyze(canonical) + self.topology_graph.metadata["execution_config"] = execution_config + self._apply_legacy_topology_shim(self.topology_graph, canonical) + self.topology_graph.validate() + self.topology_graph.validate_workflow() + def _initialize_per_topology( self, topology_graph: TopologyGraph, @@ -387,6 +412,22 @@ def _wire_event_bus(self) -> None: execution_config.aggui.queue_max_size, ) + # A rebuilt EventBus (resume_session) re-creates the listener set above, + # but the components that PUBLISH onto the bus and are REUSED across the + # rebuild — the step_executor (emits LLMCallEvent) and the user-node + # handler — still hold the prior bus. Without re-pointing them, a resumed + # run's LLM-call/user-node events publish on the stale bus, so a consumer + # re-attached via resume_session(on_bus_rebuilt=...) (e.g. a per-run cost + # adapter) never receives them. In __init__ these are created AFTER this + # call, so they're absent here and the guards no-op (they bind the fresh + # bus directly at construction); on resume they exist and get re-pointed. + step_executor = getattr(self, "step_executor", None) + if step_executor is not None and hasattr(step_executor, "event_bus"): + step_executor.event_bus = self.event_bus + user_node_handler = getattr(self, "_user_node_handler", None) + if user_node_handler is not None and hasattr(user_node_handler, "event_bus"): + user_node_handler.event_bus = self.event_bus + def _initialize_components(self): """Initialize all internal coordination components.""" # Create event bus for coordination events @@ -601,7 +642,17 @@ def _apply_legacy_topology_shim( DeprecationWarning, stacklevel=3, ) - topology_graph.register_det_node(UserNode()) + # ADR-012: a USER node may declare durability in the spec via + # node.metadata["durable"]. Read it from the CANONICAL source node + # (graph nodes drop metadata at analyze time), so a workflow + # definition can request a durable user step. Forward-compatible: + # the v0.4 generic det-node materialization reads the same metadata. + durable_user = any( + getattr(node, "kind", None) == NodeKind.USER + and bool((getattr(node, "metadata", None) or {}).get("durable")) + for node in canonical_topology.nodes + ) + topology_graph.register_det_node(UserNode(durable=durable_user)) # Legacy User-as-entry pattern: a `User → X` edge meant "workflow # starts with X receiving input from the user". In the new model, @@ -1021,25 +1072,14 @@ async def execute( self.canonical_topology.metadata = self.canonical_topology.metadata or {} self.canonical_topology.metadata["auto_inject_user"] = context.get("auto_inject_user", False) - # Build the topology graph. - self.topology_graph = self.topology_analyzer.analyze(self.canonical_topology) - self.topology_graph.metadata["execution_config"] = execution_config - - # Legacy entry/exit/User → det-node shim. Translates legacy - # entry_point/exit_points metadata and User(Node) regular nodes - # into explicit Start/End/User det-node edges. Emits - # DeprecationWarnings; full removal planned for v0.4. - self._apply_legacy_topology_shim( - self.topology_graph, self.canonical_topology - ) - - # Compile-time topology validation: runs AFTER the shim so error - # messages reference real det-nodes the user can address. Calls - # both `validate()` (det-node invariants) and `validate_workflow()` - # (workflow-completeness — every node reaches End/User; cycles - # have an escape). Skipped silently if no det-nodes registered. - self.topology_graph.validate() - self.topology_graph.validate_workflow() + # Build the topology graph (analyze + legacy entry/exit/User + # det-node shim + compile-time validation). Shared with + # resume_session via _build_topology_graph so a cross-process + # resume produces the *equivalent* graph. auto_inject_user is set + # just above from the run context; the helper's default is a no-op + # here. The shim emits DeprecationWarnings (full removal v0.4); + # validation runs after it so errors reference real det-nodes. + self._build_topology_graph(self.canonical_topology, execution_config) # Update trace with topology info now that it's analyzed. if self.trace_collector and session_id in self.trace_collector.active_traces: @@ -1130,8 +1170,23 @@ async def execute( workflow = await orchestrator.run(task=task, entry_agent=entry_agent) duration = time.time() - start_time - paused = workflow.error == "paused" - if paused: + awaiting_user = workflow.error == "awaiting_user" + paused = workflow.error == "paused" or awaiting_user + awaiting_user_prompt = None + if awaiting_user: + # Durable user wait (ADR-012): the run paused ITSELF at a durable + # interaction. Unlike the external pause_session, nothing else + # writes the snapshot — execute() must, here, while the + # orchestrator is still registered (before the finally pop). + await self._snapshot_and_write(session_id, orchestrator) + # Surface WHAT the run is awaiting on the result metadata, so a + # consumer can render the request (e.g. "re-authenticate [site]") + # without parsing the snapshot. pending_user_interaction is the + # tuple (branch_id, prompt, resume_agent, delivery_target, durable). + pending = orchestrator.pending_user_interaction + awaiting_user_prompt = pending[1] if pending else None + logger.info(f"Orchestration awaiting user after {duration:.2f}s") + elif paused: logger.info(f"Orchestration paused after {duration:.2f}s") else: logger.info(f"Orchestration completed in {duration:.2f}s") @@ -1188,6 +1243,8 @@ async def execute( "barrier_count": len(workflow.barriers), "branch_count": len(workflow.branches), "paused": paused, + "awaiting_user": awaiting_user, + "awaiting_user_prompt": awaiting_user_prompt, }, ) @@ -1314,6 +1371,23 @@ def _find_entry_agents(self) -> List[str]: logger.error(f"Failed to find entry agents: {e}") raise + async def _snapshot_and_write( + self, session_id: str, orchestrator: Orchestrator, + ) -> None: + """Build a ``StateSnapshot`` from a quiesced ``orchestrator`` and write + it atomically via the configured ``StorageBackend``. + + Shared by ``pause_session`` (external on-demand pause) and the + awaiting-user self-pause exit in ``execute()`` / ``resume_session`` + (a durable user interaction snapshots-and-exits on its own). Every + caller reaches this only when the orchestrator is no longer + dispatching (loop exited or quiesced), so ``snapshot()`` is safe. + """ + snapshot = self._build_state_snapshot(session_id, orchestrator) + payload = snapshot.model_dump_json(indent=2).encode("utf-8") + await self.storage_backend.write(self._snapshot_key(session_id), payload) + logger.info("_snapshot_and_write: wrote snapshot for session %s", session_id) + async def pause_session(self, session_id: str) -> None: """Cleanly halt the run for ``session_id`` and write a snapshot atomically. @@ -1376,12 +1450,14 @@ async def pause_session(self, session_id: str) -> None: ) return - snapshot = self._build_state_snapshot(session_id, orchestrator) - payload = snapshot.model_dump_json(indent=2).encode("utf-8") - await self.storage_backend.write(self._snapshot_key(session_id), payload) - logger.info("pause_session: wrote snapshot for session %s", session_id) + await self._snapshot_and_write(session_id, orchestrator) - async def resume_session(self, session_id: str) -> OrchestraResult: + async def resume_session( + self, session_id: str, *, + canonical_topology: "Optional[Topology]" = None, + on_bus_rebuilt: "Optional[Callable[[EventBus], None]]" = None, + user_response: Any = None, + ) -> OrchestraResult: """Read the snapshot for ``session_id`` and continue dispatch through to terminal state. @@ -1389,12 +1465,19 @@ async def resume_session(self, session_id: str) -> OrchestraResult: shape). Events flow via the existing ``EventBus`` → SSE pathway, not via the return value. - NOTE: only the standard listener set is restored on resume - (StatusManager, TraceCollector, registered TelemetrySink instances). - Custom listeners attached via ``EventBus.subscribe`` by the caller - are NOT restored; the caller must re-attach them BEFORE calling - resume_session — the snapshot read is fast, but the resumed run - begins dispatching inside this method. + ``canonical_topology`` (optional, keyword-only): a cross-process + consumer that never called ``execute()`` this process supplies the + canonical topology, and the framework binds an *equivalent* + ``topology_graph`` internally (the snapshot does not carry topology). + Omit it when the topology is already bound on this instance. + + ``on_bus_rebuilt`` (optional, keyword-only): resume rebuilds the + EventBus, restoring only the standard listener set — a caller's custom + ``EventBus.subscribe(...)`` (e.g. a per-run cost adapter) is dropped. + This callback is invoked once as ``on_bus_rebuilt(self.event_bus)`` + AFTER the resume preconditions (topology bound + digest match) pass and + BEFORE dispatch begins, so the caller re-attaches its subscribers. It + does NOT fire when resume aborts on a precondition. """ from .config import ConvergencePolicyConfig, ExecutionConfig from .execution.real_runtime import RealRuntime @@ -1423,6 +1506,23 @@ async def resume_session(self, session_id: str) -> OrchestraResult: session_id=session_id, ) + # ADR-012: durable resume-with-response argument contract (AC-5). A + # pending durable interaction REQUIRES the human's answer; an on-demand + # resume must NOT carry one. (None means "no response" — a durable + # answer is always a meaningful non-None value.) Validated early so a + # bad call fails before the EventBus/topology rebuild. + pending_interaction = snapshot.pending_user_interaction + if pending_interaction is not None and user_response is None: + raise ValueError( + f"resume_session: session {session_id!r} is awaiting a user " + f"response (durable interaction); pass user_response=… to resume." + ) + if pending_interaction is None and user_response is not None: + raise ValueError( + f"resume_session: user_response was supplied but session " + f"{session_id!r} has no pending durable user interaction." + ) + # 3. Reconstruct EventBus + listener set. self.event_bus = EventBus() self._wire_event_bus() @@ -1431,6 +1531,13 @@ async def resume_session(self, session_id: str) -> OrchestraResult: execution_config = self._execution_config or ExecutionConfig() self._execution_config = execution_config + # A cross-process consumer can supply the canonical topology so the + # framework binds an *equivalent* topology_graph internally (the + # snapshot deliberately does not carry topology). Done before the guard + # below so the consumer need not pre-set topology_graph/canonical_topology. + if canonical_topology is not None: + self._build_topology_graph(canonical_topology, execution_config) + # The topology must already be bound on this Orchestra instance. # For cross-process resume the consumer either (a) calls execute() # once to seed the topology before resume_session, or (b) sets @@ -1452,6 +1559,14 @@ async def resume_session(self, session_id: str) -> OrchestraResult: session_id=session_id, ) + # Re-attach the caller's custom EventBus subscribers on the rebuilt bus, + # now that the resume preconditions (topology bound + digest match) have + # passed. Fired here — not at the bus rebuild — so a consumer's + # subscribers never attach to a bus a failing resume discards, and a + # raising callback can't preempt the precondition errors above. + if on_bus_rebuilt is not None: + on_bus_rebuilt(self.event_bus) + # 5. Build per-topology components — same surface as execute() # constructs (validator, rules, router, agent topology refs). # Without this, RealRuntime gets validator=None and crashes on @@ -1495,6 +1610,17 @@ async def resume_session(self, session_id: str) -> OrchestraResult: orchestrator.restore_from(state) self._active_orchestrators[session_id] = orchestrator + # ADR-012: inject the human's response into the suspended branch BEFORE + # dispatch resumes. Reuses the existing public seam, which spawns the + # resume_agent branch with the response as input, terminates the + # suspended branch, and clears the pending-interaction scalar. + if pending_interaction is not None: + orchestrator.resume_branch_with_user_response( + pending_interaction.suspended_branch_id, + user_response, + pending_interaction.resume_agent, + ) + logger.info("resume_session: resuming session %s", session_id) start_time = time.time() # If anything is added to this finally that reads a try-scoped @@ -1511,12 +1637,16 @@ async def resume_session(self, session_id: str) -> OrchestraResult: self._active_orchestrators.pop(session_id, None) duration = time.time() - start_time - paused_again = workflow.error == "paused" + awaiting_user_again = workflow.error == "awaiting_user" + paused_again = workflow.error == "paused" or awaiting_user_again + awaiting_user_prompt_again = None total_steps = sum(b.step_count for b in workflow.branches.values()) - # 8. On successful terminal state, discard the snapshot. Failed - # resumes leave the snapshot in place for inspection — log a - # warning so the operator knows. + # 8. Terminal success → discard the snapshot. Awaiting-user again (a + # resumed run that hit ANOTHER durable interaction, ADR-012) → re-write + # the snapshot with the new suspended state. An external on-demand pause + # during resume already wrote its own snapshot via pause_session. Other + # non-paused failures → leave the snapshot for inspection. if workflow.success: try: await self.storage_backend.delete(key) @@ -1525,6 +1655,10 @@ async def resume_session(self, session_id: str) -> OrchestraResult: "resume_session: failed to delete snapshot for %s: %s", session_id, exc, ) + elif awaiting_user_again: + await self._snapshot_and_write(session_id, orchestrator) + pending = orchestrator.pending_user_interaction + awaiting_user_prompt_again = pending[1] if pending else None elif not paused_again: logger.warning( "resume_session: %s ended in error=%r; snapshot left in place " @@ -1560,6 +1694,8 @@ async def resume_session(self, session_id: str) -> OrchestraResult: "session_id": session_id, "resumed": True, "paused": paused_again, + "awaiting_user": awaiting_user_again, + "awaiting_user_prompt": awaiting_user_prompt_again, "barrier_count": len(workflow.barriers), "branch_count": len(workflow.branches), }, @@ -1680,6 +1816,10 @@ def _build_state_snapshot( for item in state.user_interactions ], user_interaction_inflight=state.user_interaction_inflight, + pending_user_interaction=( + self._user_interaction_to_state(state.pending_user_interaction) + if state.pending_user_interaction is not None else None + ), max_steps=state.max_steps, ) try: @@ -1746,14 +1886,15 @@ def _barrier_to_state(bar: "Barrier") -> BarrierState: @staticmethod def _user_interaction_to_state(item: tuple) -> UserInteractionState: - # Items are (suspended_branch_id, prompt, resume_agent, delivery_target) - # tuples per Orchestrator.enqueue_user_interaction. - bid, prompt, resume_agent, delivery_target = item + # Items are (suspended_branch_id, prompt, resume_agent, delivery_target, + # durable) tuples per Orchestrator.enqueue_user_interaction. + bid, prompt, resume_agent, delivery_target, durable = item return UserInteractionState( suspended_branch_id=bid, prompt=prompt, resume_agent=resume_agent, delivery_target=delivery_target, + durable=durable, ) def _snapshot_to_orchestrator_state( @@ -1809,9 +1950,16 @@ def _snapshot_to_orchestrator_state( for barid, s in snapshot.barriers.items() } user_interactions = [ - (ui.suspended_branch_id, ui.prompt, ui.resume_agent, ui.delivery_target) + (ui.suspended_branch_id, ui.prompt, ui.resume_agent, ui.delivery_target, + ui.durable) for ui in snapshot.user_interactions ] + pending = snapshot.pending_user_interaction + pending_user_interaction = ( + (pending.suspended_branch_id, pending.prompt, pending.resume_agent, + pending.delivery_target, pending.durable) + if pending is not None else None + ) return OrchestratorState( branches=branches, barriers=barriers, @@ -1823,6 +1971,7 @@ def _snapshot_to_orchestrator_state( completed_emitted=set(snapshot.completed_emitted), user_interactions=user_interactions, user_interaction_inflight=snapshot.user_interaction_inflight, + pending_user_interaction=pending_user_interaction, max_steps=snapshot.max_steps, ) diff --git a/src/marsys/coordination/state/snapshot.py b/src/marsys/coordination/state/snapshot.py index 77e7605a..9efe6f85 100644 --- a/src/marsys/coordination/state/snapshot.py +++ b/src/marsys/coordination/state/snapshot.py @@ -95,6 +95,9 @@ class UserInteractionState(BaseModel): prompt: Any resume_agent: str delivery_target: str + # ADR-012: True for a durable interaction (snapshot-and-exit), False for the + # SYNC path. Default False keeps pre-ADR-012 snapshots valid under extra=forbid. + durable: bool = False class StateSnapshot(BaseModel): @@ -123,6 +126,11 @@ class StateSnapshot(BaseModel): completed_emitted: list[str] user_interactions: list[UserInteractionState] user_interaction_inflight: bool + # The single in-flight DURABLE user interaction (ADR-012), held apart from + # the queued-siblings `user_interactions` deque (whose FIFO pop would + # mis-dispatch it as a sibling). None for SYNC interactions and for + # snapshots written before this field existed (extra='forbid'-safe). + pending_user_interaction: Optional[UserInteractionState] = None max_steps: int = 200 # mirrors Orchestrator.max_steps; preserved across resume diff --git a/src/marsys/coordination/validation/response_validator.py b/src/marsys/coordination/validation/response_validator.py index 9563fa3a..3bddcadc 100644 --- a/src/marsys/coordination/validation/response_validator.py +++ b/src/marsys/coordination/validation/response_validator.py @@ -35,6 +35,7 @@ class ActionType(Enum): FINAL_RESPONSE = "final_response" TERMINATE_WORKFLOW = "terminate_workflow" ASK_USER = "ask_user" + ESCALATE_USER = "escalate_user" END_CONVERSATION = "end_conversation" ERROR_RECOVERY = "error_recovery" TERMINAL_ERROR = "terminal_error" @@ -114,6 +115,8 @@ async def validate_coordination_action( return await self._validate_return_final_response(data, agent) elif action == "ask_user": return await self._validate_ask_user(data, agent) + elif action == "escalate_to_user": + return await self._validate_escalate_user(data, agent) elif action == "end_conversation": return await self._validate_end_conversation(data, agent, branch) else: @@ -250,6 +253,41 @@ async def _validate_ask_user( }, ) + async def _validate_escalate_user( + self, data: Dict[str, Any], agent: BaseAgent + ) -> ValidationResult: + """Validate escalate_to_user: the agent must be granted `can_escalate`. + + Unlike ask_user (gated on a topology edge to a User det-node), the + escalate directive is gated on a per-agent capability — it suspends the + run for a human WITHOUT a topology User node (ADR-013). Same gate axis at + both the schema offer (can_escalate_user) and here.""" + if not getattr(agent, "can_escalate", False): + next_agents = self.topology_graph.get_next_agents(agent.name) + return ValidationResult( + is_valid=False, + error_message=f"Agent '{agent.name}' cannot escalate to the user (not granted can_escalate)", + retry_suggestion=f"You are not permitted to escalate to the user. Continue with one of: {next_agents}", + error_category=ValidationErrorCategory.PERMISSION_ERROR.value, + ) + + prompt = data.get("prompt", "") + if not prompt: + return ValidationResult( + is_valid=False, + error_message="escalate_to_user called with empty prompt", + error_category=ValidationErrorCategory.ACTION_ERROR.value, + ) + + return ValidationResult( + is_valid=True, + action_type=ActionType.ESCALATE_USER, + parsed_response={ + "prompt": prompt, + "action_input": {"prompt": prompt}, + }, + ) + async def _validate_end_conversation( self, data: Dict[str, Any], agent: BaseAgent, branch: ExecutionBranch ) -> ValidationResult: diff --git a/src/marsys/models/adapters/__init__.py b/src/marsys/models/adapters/__init__.py index 7d08e300..07a154f5 100644 --- a/src/marsys/models/adapters/__init__.py +++ b/src/marsys/models/adapters/__init__.py @@ -4,6 +4,7 @@ from marsys.models.adapters.openai import OpenAIAdapter, AsyncOpenAIAdapter from marsys.models.adapters.openrouter import OpenRouterAdapter, AsyncOpenRouterAdapter from marsys.models.adapters.anthropic import AnthropicAdapter, AsyncAnthropicAdapter +from marsys.models.adapters.bedrock import AsyncBedrockAdapter, BedrockAdapter from marsys.models.adapters.google import GoogleAdapter, AsyncGoogleAdapter from marsys.models.adapters.openai_oauth import OpenAIOAuthAdapter, AsyncOpenAIOAuthAdapter from marsys.models.adapters.anthropic_oauth import AnthropicOAuthAdapter, AsyncAnthropicOAuthAdapter @@ -29,6 +30,9 @@ # Anthropic "AnthropicAdapter", "AsyncAnthropicAdapter", + # Bedrock (Claude on Amazon Bedrock) + "BedrockAdapter", + "AsyncBedrockAdapter", # Google "GoogleAdapter", "AsyncGoogleAdapter", diff --git a/src/marsys/models/adapters/anthropic.py b/src/marsys/models/adapters/anthropic.py index dd78102c..e3af976a 100644 --- a/src/marsys/models/adapters/anthropic.py +++ b/src/marsys/models/adapters/anthropic.py @@ -26,33 +26,139 @@ logger = logging.getLogger(__name__) -def _anthropic_model_rejects_temperature(model_name: str) -> bool: - """Return True for Anthropic models that reject the `temperature` - parameter on the messages API. - - Claude Opus 4.7 (and its 1M-context variants) treats `temperature` - as deprecated and 400s the request when it is set. The shape of - Anthropic's deprecation has been "reasoning-capable models drop - sampling parameters," so any future Opus 4.x line is expected to - behave the same way; we match by the documented prefix and let the - request fail loudly for a model name we have not seen yet. +def _normalize_anthropic_model(model_name: str) -> str: + """Bare, lower-cased model id for capability matching. + + The same model arrives under several spellings: an ``anthropic/`` prefix + (OpenRouter), an ``anthropic.`` / ``us.anthropic.`` prefix (Bedrock), and + with or without a date suffix. Capability is a property of the model, not + of the spelling, so all of them collapse to one key here. """ - if not model_name: + name = (model_name or "").lower() + for prefix in ("us.anthropic.", "eu.anthropic.", "apac.anthropic.", "anthropic.", "anthropic/"): + if name.startswith(prefix): + name = name[len(prefix):] + break + return name + + +# Reasoning-capable Claude models progressively dropped the sampling parameters +# and the fixed thinking budget. Both are hard 400s, not ignored fields, so the +# payload has to be shaped per model. Measured against the live API (Bedrock and +# the OAuth/Messages endpoint): `temperature` is rejected by every model below, +# and `thinking.type="enabled"` is rejected in favour of +# `thinking.type="adaptive"` + `output_config.effort`. Matching is by prefix so +# dated snapshots and 1M-context variants inherit the capability; a model we +# have not seen yet keeps the legacy shape and fails loudly rather than silently. +_ADAPTIVE_THINKING_MODEL_PREFIXES = ( + "claude-opus-4-7", + "claude-opus-4-8", + "claude-opus-5", + "claude-sonnet-5", + "claude-fable-5", + "claude-mythos-5", +) + + +def _anthropic_model_rejects_temperature(model_name: str) -> bool: + """True for models that 400 when `temperature` is present.""" + name = _normalize_anthropic_model(model_name) + if not name: + return False + return name.startswith(_ADAPTIVE_THINKING_MODEL_PREFIXES) + + +def _anthropic_model_requires_adaptive_thinking(model_name: str) -> bool: + """True for models where a fixed `budget_tokens` is rejected and thinking + is requested as `{"type": "adaptive"}` instead.""" + name = _normalize_anthropic_model(model_name) + if not name: return False - # Anthropic ships model names with or without the "anthropic/" - # prefix (OpenRouter etc.); strip it before comparing. - name = model_name.lower() - if name.startswith("anthropic/"): - name = name[len("anthropic/"):] - return ( - name.startswith("claude-opus-4-7") - or name.startswith("claude-opus-4-8") - ) + return name.startswith(_ADAPTIVE_THINKING_MODEL_PREFIXES) + + +# Block types the API accepts a `cache_control` marker on. A marker on anything +# else is rejected, so an unrecognized tail block is skipped rather than guessed +# at — a missed cache entry costs money, an illegal field costs the whole turn. +_CACHEABLE_BLOCK_TYPES = frozenset( + {"text", "image", "tool_use", "tool_result", "document"} +) + +CACHE_CONTROL_EPHEMERAL = {"type": "ephemeral"} + + +def mark_conversation_tail_for_cache(messages: List[Dict[str, Any]]) -> None: + """Place ONE prompt-cache breakpoint on the last content block of the last + message, in place on ``messages`` — the platform's multi-turn caching pattern. + + Adapter-owned and unconditional, matching the only other `cache_control` site + in this codebase (the OAuth adapter's static Claude-Code prefix block). Only + the payload builder knows the rendered block layout, and caching is prefix-match + arithmetic over exactly those bytes, so a caller cannot place this correctly + even if it wanted to — and a caller that forgets silently re-pays full price on + the whole conversation. The precedent is `defer_loading`: the framework's + nearest analogous feature deliberately took no new request parameter either. + + Why the TAIL and why EVERY request: a breakpoint reads any entry written at or + before it, so marking the growing tail each time both reads the previous + request's entry and extends it by that turn's new blocks. It also satisfies the + 20-block lookback window by construction — a per-request tail marker is always + a handful of blocks behind the last one, whereas a marker placed once silently + stops matching in an agentic turn that appends several blocks per step. + + Never mutates a caller block dict: the durable conversation shares those dicts + (the same hazard `hydrate_messages` documents), so a marker stamped in place + would leak into persisted rows. The message's content list and the marked block + are copied instead — which also makes the placement idempotent, since building + a payload twice from the same input yields byte-identical output. + + No-ops (leaving the payload byte-identical to the unmarked form) when there is + nothing safe to mark: an empty message list, empty content, a tail message that + already carries a marker, or a tail with no cacheable block type. A prompt under + the model's cacheable minimum silently writes nothing and costs nothing, so no + size check is needed here. + """ + if not messages: + return + last = messages[-1] + content = last.get("content") + + if isinstance(content, str): + # Promote to a one-block list so the marker has a block to ride. An empty + # string is left alone: the API rejects an empty text block, and a bare + # empty string is what this adapter already sends for a contentless message. + if not content: + return + last["content"] = [ + {"type": "text", "text": content, "cache_control": dict(CACHE_CONTROL_EPHEMERAL)} + ] + return + + if not isinstance(content, list) or not content: + return + # Idempotence + the 4-breakpoint budget: a message that already carries a + # marker never receives a second one. + if any(isinstance(b, dict) and b.get("cache_control") for b in content): + return + for index in range(len(content) - 1, -1, -1): + block = content[index] + if isinstance(block, dict) and block.get("type") in _CACHEABLE_BLOCK_TYPES: + marked = {**block, "cache_control": dict(CACHE_CONTROL_EPHEMERAL)} + new_content = list(content) + new_content[index] = marked + last["content"] = new_content + return class AnthropicAdapter(APIProviderAdapter): """Adapter for Anthropic Claude API""" + # Endpoint capability, not model capability: the first-party Messages API + # enforces `output_config.format`, while the Bedrock endpoints reject the + # key outright. Subclasses that speak to an endpoint without it flip this + # to False and inherit the prompt-based fallback. + supports_structured_output = True + def __init__( self, model_name: str, @@ -80,6 +186,15 @@ def get_headers(self) -> Dict[str, str]: "anthropic-version": "2023-06-01", } + def report_model_id(self, echoed: Optional[str]) -> str: + """Which model id to report on the response metadata. + + The provider echo is preferred because it resolves an alias to the + concrete snapshot actually served. Subclasses whose endpoint echoes an id + in a *different namespace* than the one it accepts override this. + """ + return echoed or self.model_name + def _convert_content_to_anthropic_format(self, content: Any) -> Any: """ Convert OpenAI-style image content to Anthropic format. @@ -172,6 +287,12 @@ def _thinking_payload(self, kwargs: Dict[str, Any]) -> Optional[Dict[str, Any]]: budget = kwargs.get("thinking_budget") if not isinstance(budget, int) or budget <= 0: return None + if _anthropic_model_requires_adaptive_thinking(self.model_name): + # These models reject a fixed budget; the model decides depth itself. + # A positive budget keeps its framework meaning ("thinking on") and + # the size is dropped — depth is steered by `output_config.effort`, + # which rides `reasoning_effort` when a caller sets it. + return {"type": "adaptive"} max_tokens = kwargs.get("max_tokens") or self.max_tokens clamped = min(budget, max_tokens - self._THINKING_HEADROOM) if clamped < self._THINKING_MIN_BUDGET: @@ -301,23 +422,63 @@ def format_request_payload(self, messages: List[Dict], **kwargs) -> Dict[str, An if thinking_payload is not None: payload["thinking"] = thinking_payload + # Thinking depth on adaptive-thinking models is steered by effort, which + # replaced the fixed budget. Only sent for models that accept it (older + # models 400 on the key), and never alongside disabled thinking: Opus 5 + # rejects effort above "high" when thinking is off, and the combination + # buys nothing anyway. + effort = kwargs.get("reasoning_effort") + if ( + effort + and thinking_payload is not None + and _anthropic_model_requires_adaptive_thinking(self.model_name) + ): + payload.setdefault("output_config", {})["effort"] = str(effort).lower() + if system_message: - payload["system"] = system_message + # ARRAY form, not a bare string. Both are accepted and carry identical + # text to the model, but only the array form has content blocks a + # `cache_control` marker could ever ride — the string form makes the + # system tier structurally unmarkable. No marker is placed here yet: + # on this leg the system content is the caller's per-turn prompt, which + # for Spren's six-axis overview changes every turn (a timestamp line and + # a budget line), so a marker here would write a fresh entry per call and + # read none — pure write premium. The shape lands now so the marker is a + # one-line change once that prompt is made byte-stable. + payload["system"] = ( + [{"type": "text", "text": system_message}] + if isinstance(system_message, str) + else system_message + ) - # Handle structured output — native output_config.format (GA) + # Handle structured output — native output_config.format (GA). + # `supports_structured_output` is False where the endpoint rejects the + # key (Bedrock); there the schema degrades to the prompt-based fallback + # below rather than putting an illegal field on the wire. response_schema = kwargs.get("response_schema") - if response_schema: - payload["output_config"] = { - "format": { - "type": "json_schema", - "schema": self._ensure_additional_properties_false(response_schema) - } + if response_schema and self.supports_structured_output: + # Merge, never assign: `effort` may already own output_config, and + # the API takes exactly one such object per request. + payload.setdefault("output_config", {})["format"] = { + "type": "json_schema", + "schema": self._ensure_additional_properties_false(response_schema), } - elif kwargs.get("json_mode") and user_messages: - # No native json_object mode in Anthropic — use prompt-based fallback + elif (kwargs.get("json_mode") or response_schema) and user_messages: + # No native json_object mode in Anthropic — use prompt-based fallback. + # When a schema was requested but the endpoint cannot enforce it, the + # schema goes into the prompt: a bare "valid JSON" hint would satisfy + # the caller's parser only by luck. last_msg = user_messages[-1] if last_msg.get("role") == "user": hint = "\n\nPlease respond with valid JSON only." + if response_schema: + hint = ( + "\n\nRespond with valid JSON only — no prose, no code fence — " + "conforming exactly to this JSON Schema:\n" + + json.dumps( + self._ensure_additional_properties_false(response_schema) + ) + ) content = last_msg["content"] if isinstance(content, list): last_msg["content"] = content + [{"type": "text", "text": hint}] @@ -327,24 +488,53 @@ def format_request_payload(self, messages: List[Dict], **kwargs) -> Dict[str, An # Handle tools - convert OpenAI format to Anthropic format # OpenAI: {"type": "function", "function": {"name": ..., "description": ..., "parameters": ...}} # Anthropic: {"name": ..., "description": ..., "input_schema": ...} + # A per-tool ``defer_loading: true`` (deferred tool loading) rides the OpenAI tool dict + # at the top level; it maps onto the Anthropic tool and triggers the Tool Search server + # tool so the model discovers deferred tools on demand — their schemas stay out of the + # billed/cached prefix until searched. With nothing deferred this branch is byte-identical + # to before (no defer_loading key emitted, no search tool added). if kwargs.get("tools"): anthropic_tools = [] + any_deferred = False for tool in kwargs["tools"]: if isinstance(tool, dict): if tool.get("type") == "function" and "function" in tool: # Convert from OpenAI format func = tool["function"] - anthropic_tools.append({ + converted = { "name": func.get("name"), "description": func.get("description", ""), "input_schema": func.get("parameters", {"type": "object", "properties": {}}) - }) + } + if tool.get("defer_loading"): + converted["defer_loading"] = True + any_deferred = True + anthropic_tools.append(converted) elif "name" in tool and "input_schema" in tool: - # Already in Anthropic format + # Already in Anthropic format (incl. a pre-marked defer_loading tool or a + # caller-supplied tool-search server tool) — pass through verbatim. anthropic_tools.append(tool) + if tool.get("defer_loading"): + any_deferred = True + if any_deferred and not any( + isinstance(t, dict) and str(t.get("type", "")).startswith("tool_search_tool") + for t in anthropic_tools + ): + # Auto-add the Tool Search server tool (regex variant) so deferred tools are + # discoverable. It is non-deferred by construction (the API requires >=1 + # non-deferred tool). Suppressed if the caller supplied their own search tool. + anthropic_tools.append( + {"type": "tool_search_tool_regex_20251119", "name": "tool_search_tool_regex"} + ) if anthropic_tools: payload["tools"] = anthropic_tools + # LAST, deliberately: the marker belongs on the final content block of the + # final message, and the json-mode fallback above may still append a hint + # block there. Placing it after every content mutation is what makes "the + # tail" mean the actual tail. + mark_conversation_tail_for_cache(user_messages) + return payload def get_endpoint_url(self) -> str: @@ -444,6 +634,13 @@ def harmonize_response( completion_tokens=usage_data.get("output_tokens"), total_tokens=usage_data.get("input_tokens", 0) + usage_data.get("output_tokens", 0), + # Prompt-cache accounting. `input_tokens` is the UNCACHED + # remainder, so a caller measuring the whole prompt needs these + # two alongside it (UsageInfo.full_prompt_tokens). Absent on a + # response that reports no cache activity → None, and + # total_tokens keeps its established meaning either way. + cache_read_input_tokens=usage_data.get("cache_read_input_tokens"), + cache_creation_input_tokens=usage_data.get("cache_creation_input_tokens"), ) # Build metadata with Anthropic-specific fields. finish_reason carries the @@ -461,8 +658,13 @@ def harmonize_response( else stop_reason_raw ) metadata = ResponseMetadata( - provider="anthropic", - model=raw_response.get("model", self.model_name), + provider=self._provider_name() or "anthropic", + # `metadata.model` is what cost meters price on, so it must be the id + # the caller's rate table is keyed by. Bedrock echoes a *bare* id for + # a request made with an `anthropic.`-prefixed one, so trusting the + # echo silently prices that whole provider at zero. `report_model_id` + # keeps the requested spelling where the echo would not round-trip. + model=self.report_model_id(raw_response.get("model")), request_id=raw_response.get("id"), usage=usage, finish_reason=finish_reason, @@ -473,8 +675,9 @@ def harmonize_response( # Empty-output contract (twin of anthropic_oauth.py): deterministic # truncation gets the cross-adapter placeholder (openai.py's convention) - # so callers see one shape, never None; every OTHER fully-empty terminal - # (refusal / empty end_turn / no stop_reason) raises a typed + # so callers see one shape, never None; a natural-completion terminal + # (end_turn) is a SILENT TURN and takes the content="" path below; every + # OTHER fully-empty terminal (refusal / no stop_reason) raises a typed # ModelAPIError classified by stop_reason instead of constructing a # content=None shell the model validator rejects as an UNKNOWN # ValidationError. Thinking-only responses are NOT empty — they take the @@ -494,7 +697,7 @@ def harmonize_response( "[Response truncated due to token limit. Please increase max_tokens " "or continue the conversation.]" ) - else: + elif stop_reason_raw != "end_turn": from marsys.agents.exceptions import ModelAPIError raise ModelAPIError.from_provider_response( @@ -503,12 +706,14 @@ def harmonize_response( ) content = text_content if text_content else None - # Thinking-only response (the latent gap anthropic_oauth.py:766 records, - # reachable now that thinking is enableable): the validator requires - # content-or-tool_calls and ignores thinking. An empty STRING is a valid - # content shape (the None check is what fails), so a response that is - # all thinking harmonizes instead of dying in validation. - if content is None and not tool_calls and (thinking_parts or reasoning_details): + # An empty STRING is a valid content shape (the validator's None check is + # what fails), so two responses that carry no text still harmonize rather + # than dying in validation: a thinking-only response, and a SILENT TURN — + # the model ran to natural completion (end_turn) and chose to produce + # nothing, which callers ask for and the provider bills as a success. + if content is None and not tool_calls and ( + thinking_parts or reasoning_details or stop_reason_raw == "end_turn" + ): content = "" # Build harmonized response diff --git a/src/marsys/models/adapters/anthropic_oauth.py b/src/marsys/models/adapters/anthropic_oauth.py index 4d3f9e1f..d78f4c12 100644 --- a/src/marsys/models/adapters/anthropic_oauth.py +++ b/src/marsys/models/adapters/anthropic_oauth.py @@ -6,6 +6,11 @@ from pathlib import Path from typing import Any, Dict, List, Optional +from marsys.models.adapters.anthropic import ( + _anthropic_model_rejects_temperature, + _anthropic_model_requires_adaptive_thinking, + mark_conversation_tail_for_cache, +) from marsys.models.adapters.base import APIProviderAdapter, AsyncBaseAPIAdapter from marsys.models.response_models import ( ErrorResponse, @@ -55,6 +60,8 @@ class AnthropicOAuthAdapter(APIProviderAdapter): # Supported models SUPPORTED_MODELS = [ + "claude-opus-5", + "claude-sonnet-5", "claude-opus-4-8", "claude-opus-4-7", "claude-opus-4-6", @@ -68,6 +75,8 @@ class AnthropicOAuthAdapter(APIProviderAdapter): # Model aliases for convenience (OpenRouter convention with dots) MODEL_ALIASES = { # OpenRouter-style aliases (with dots) + "claude-opus-5.0": "claude-opus-5", + "claude-sonnet-5.0": "claude-sonnet-5", "claude-opus-4.8": "claude-opus-4-8", "claude-opus-4.7": "claude-opus-4-7", "claude-opus-4.6": "claude-opus-4-6", @@ -82,8 +91,8 @@ class AnthropicOAuthAdapter(APIProviderAdapter): "claude-haiku-4-5": "claude-haiku-4-5-20251001", "claude-opus-4-1": "claude-opus-4-1-20250805", # Short aliases - "opus": "claude-opus-4-8", - "sonnet": "claude-sonnet-4-6", + "opus": "claude-opus-5", + "sonnet": "claude-sonnet-5", "haiku": "claude-haiku-4-5-20251001", } @@ -506,38 +515,73 @@ def format_request_payload(self, messages: List[Dict], **kwargs) -> Dict[str, An "stream": True, # Always stream for OAuth } - # Add temperature if provided + # Thinking first: it decides whether sampling params are legal at all. + # A positive budget means "thinking on" (the framework convention that + # BaseAPIModel.arun injects); reasoning-capable models take + # `{"type": "adaptive"}` and reject a fixed budget, so the size is + # dropped for them and depth rides `output_config.effort` instead. + thinking_on = bool(self.enable_thinking or kwargs.get("enable_thinking")) + budget = kwargs.get("thinking_budget", self.thinking_budget) + if not thinking_on and isinstance(budget, int) and budget > 0: + thinking_on = True + if thinking_on: + if _anthropic_model_requires_adaptive_thinking(self.model_name): + payload["thinking"] = {"type": "adaptive"} + effort = kwargs.get("reasoning_effort") + if effort: + payload.setdefault("output_config", {})["effort"] = str(effort).lower() + else: + payload["thinking"] = {"type": "enabled", "budget_tokens": budget} + + # Temperature only when the model accepts it AND thinking is off — the + # reasoning-capable models 400 on the key ("`temperature` is deprecated + # for this model."), and thinking forbids sampling params outright. temperature = kwargs.get("temperature", self.temperature) - if temperature is not None: + if ( + temperature is not None + and "thinking" not in payload + and not _anthropic_model_rejects_temperature(self.model_name) + ): payload["temperature"] = temperature - # Add thinking if enabled - if self.enable_thinking or kwargs.get("enable_thinking"): - payload["thinking"] = { - "type": "enabled", - "budget_tokens": kwargs.get("thinking_budget", self.thinking_budget) - } - - # Convert tools to Anthropic format with reserved name transformation + # Convert tools to Anthropic format with reserved name transformation. + # A per-tool ``defer_loading: true`` rides the tool dict top-level (deferred tool loading); + # it maps onto the Anthropic tool and triggers the Tool Search server tool so deferred + # tools are discovered on demand (their schemas stay out of the cached prefix). Nothing + # deferred → byte-identical to before (no defer_loading key, no search tool). if kwargs.get("tools"): anthropic_tools = [] + any_deferred = False for tool in kwargs["tools"]: if tool.get("type") == "function" and "function" in tool: func = tool["function"] original_name = func.get("name", "") api_name = self._transform_tool_name_for_api(original_name) - anthropic_tools.append({ + converted = { "name": api_name, "description": func.get("description", ""), "input_schema": func.get("parameters", {"type": "object", "properties": {}}) - }) + } + if tool.get("defer_loading"): + converted["defer_loading"] = True + any_deferred = True + anthropic_tools.append(converted) elif "name" in tool and "input_schema" in tool: # Already in Anthropic format - still transform the name original_name = tool.get("name", "") api_name = self._transform_tool_name_for_api(original_name) transformed_tool = {**tool, "name": api_name} anthropic_tools.append(transformed_tool) - + if tool.get("defer_loading"): + any_deferred = True + + if any_deferred and not any( + isinstance(t, dict) and str(t.get("type", "")).startswith("tool_search_tool") + for t in anthropic_tools + ): + anthropic_tools.append( + {"type": "tool_search_tool_regex_20251119", "name": "tool_search_tool_regex"} + ) if anthropic_tools: payload["tools"] = anthropic_tools @@ -561,6 +605,14 @@ def format_request_payload(self, messages: List[Dict], **kwargs) -> Dict[str, An elif isinstance(content, str): last_msg["content"] = content + hint + # The conversation-tail prompt-cache breakpoint (mirrors the api-key twin; + # see ``mark_conversation_tail_for_cache``). Placed LAST, after the json-mode + # hint above, so it lands on the real tail block. This is the SECOND marker + # in an OAuth payload — the static Claude-Code prefix block in + # ``_build_system_array`` is the first — which keeps the payload two under + # the API's four-breakpoint ceiling. + mark_conversation_tail_for_cache(converted_messages) + return payload def _sync_stream_response( @@ -612,6 +664,11 @@ def _sync_stream_response( msg = data.get("message", {}) result["model"] = msg.get("model") result["id"] = msg.get("id") + # Usage is MERGED across events, never assigned (see + # the async twin's arm for the full reasoning). + start_usage = msg.get("usage") + if isinstance(start_usage, dict): + result["usage"].update(start_usage) elif event_type == "content_block_start": block = data.get("content_block", {}) @@ -645,7 +702,9 @@ def _sync_stream_response( # Nullable decoration on the terminal (refusal # category etc.) — for error messages, never keyed on. result["stop_details"] = delta.get("stop_details") - result["usage"] = data.get("usage", {}) + delta_usage = data.get("usage") + if isinstance(delta_usage, dict): + result["usage"].update(delta_usage) elif event_type == "error": # Anthropic delivers stream failures as in-stream SSE error @@ -732,6 +791,20 @@ def _tap(kind: str, delta: str) -> None: msg = data.get("message", {}) result["model"] = msg.get("model") result["id"] = msg.get("id") + # Usage is MERGED across events, never assigned. The + # grammar splits it: `message_start` is the only event + # carrying the cache-TTL breakdown (`cache_creation`), + # `service_tier` and `inference_geo`, while + # `message_delta` carries the final `output_tokens`. + # Assigning at `message_delta` therefore dropped + # everything only `message_start` reports, and a stream + # that ends WITHOUT a `message_delta` (an in-stream + # error after prefill) harmonized with no usage at all. + # Merging matches AnthropicStreamAccumulator, the + # api-key leg's shared accumulator. + start_usage = msg.get("usage") + if isinstance(start_usage, dict): + result["usage"].update(start_usage) elif event_type == "content_block_start": block = data.get("content_block", {}) @@ -768,7 +841,9 @@ def _tap(kind: str, delta: str) -> None: result["stop_reason"] = delta.get("stop_reason") # Twin of the sync reader: nullable terminal decoration. result["stop_details"] = delta.get("stop_details") - result["usage"] = data.get("usage", {}) + delta_usage = data.get("usage") + if isinstance(delta_usage, dict): + result["usage"].update(delta_usage) elif event_type == "error": # See the sync reader's twin arm: in-stream SSE failure under @@ -804,15 +879,19 @@ def harmonize_response( """Convert streaming response to HarmonizedResponse. Empty-output contract: a stream that terminated with NO text, NO tool - calls, and NO thinking either takes the truncation placeholder + calls, and NO thinking takes one of three arms. Deterministic truncation (normalized finish_reason ``length`` — ``max_tokens`` or - ``model_context_window_exceeded``) or raises a typed, classified - ``ModelAPIError`` built from the terminal signal (``refusal``, empty - ``end_turn``, no terminal at all). Together with the run paths' - in-stream ``error`` handling, every stream outcome maps to a valid - ``HarmonizedResponse`` or a typed ``ModelAPIError`` — never a - ``content=None`` shell that dies in the model validator as an - UNKNOWN ValidationError with the provider signal destroyed. + ``model_context_window_exceeded``) gets the truncation placeholder. A + natural-completion terminal (``end_turn``) is a SILENT TURN: the model + finished and chose to say nothing, which is a success, and harmonizes to + the empty-string content shape (the validator rejects ``None``, not + ``""``). Every OTHER empty terminal (``refusal``, no terminal at all) + raises a typed, classified ``ModelAPIError`` built from the terminal + signal. Together with the run paths' in-stream ``error`` handling, every + stream outcome maps to a valid ``HarmonizedResponse`` or a typed + ``ModelAPIError`` — never a ``content=None`` shell that dies in the model + validator as an UNKNOWN ValidationError with the provider signal + destroyed. (Latent gap, recorded 2026-06-11, still open: a thinking-only response — ``thinking`` set, no text, no tool calls, NON-length stop_reason — is @@ -846,6 +925,11 @@ def harmonize_response( usage = UsageInfo( prompt_tokens=usage_data.get("input_tokens"), completion_tokens=usage_data.get("output_tokens"), + # Prompt-cache accounting — see the api-key twin. `input_tokens` is + # the uncached remainder; the whole prompt is + # UsageInfo.full_prompt_tokens. + cache_read_input_tokens=usage_data.get("cache_read_input_tokens"), + cache_creation_input_tokens=usage_data.get("cache_creation_input_tokens"), ) if usage_data else None # Build metadata. finish_reason carries the NORMALIZED vocabulary (the @@ -874,14 +958,24 @@ def harmonize_response( # Empty-output contract (docstring above). Deterministic truncation gets # the cross-adapter placeholder (openai.py's convention) so callers see - # one shape, never None; every OTHER empty terminal is a typed failure - # classified by stop_reason (refusal / empty end_turn / no terminal). + # one shape, never None; a natural-completion terminal is a silent turn; + # every OTHER empty terminal is a typed failure classified by stop_reason + # (refusal / no terminal). + content = text_content if text_content else None if not text_content and not tool_calls and not raw_response.get("thinking"): if finish_reason == "length": - text_content = ( + content = ( "[Response truncated due to token limit. Please increase max_tokens " "or continue the conversation.]" ) + elif stop_reason_raw == "end_turn": + # A silent turn: the model ran to natural completion and produced + # nothing. Callers ask for this (an agent told to stay quiet when + # it has nothing to report), the provider bills it as a success, + # and the empty STRING is the content shape that carries it — the + # validator's rejection is of None, never of "". The API-key twin + # uses the same escape for its thinking-only responses. + content = "" else: from marsys.agents.exceptions import ModelAPIError from marsys.models.adapters.streaming import empty_completion_payload @@ -894,7 +988,7 @@ def harmonize_response( # Build response return HarmonizedResponse( role="assistant", - content=text_content if text_content else None, + content=content, tool_calls=tool_calls, thinking=raw_response.get("thinking") or None, metadata=metadata, diff --git a/src/marsys/models/adapters/bedrock.py b/src/marsys/models/adapters/bedrock.py new file mode 100644 index 00000000..a9bd5e4b --- /dev/null +++ b/src/marsys/models/adapters/bedrock.py @@ -0,0 +1,139 @@ +"""Adapter for Claude on Amazon Bedrock. + +Bedrock exposes two shapes for the same models. This adapter targets the +**Messages-API-shaped** endpoint (``bedrock-mantle..api.aws``), which +speaks the identical request/response body as the first-party Messages API and +streams real SSE. That is what makes this a thin subclass of +:class:`AnthropicAdapter` instead of a parallel implementation: message +conversion, thinking/temperature capability shaping, tool conversion, deferred +tool loading, SSE accumulation and harmonization are all inherited. + +The other shape — ``bedrock-runtime``'s ``/invoke`` and +``/invoke-with-response-stream`` — takes ``anthropic_version`` in the body, no +``model`` key, and frames streams as AWS binary ``application/vnd.amazon.eventstream`` +rather than SSE. Using it would mean a second stream parser for no gain, so it is +deliberately not used. + +Three deltas from the first-party adapter, each measured against the live +endpoint rather than assumed: + +* **Auth** — a bearer token (``AWS_BEARER_TOKEN_BEDROCK``), not ``x-api-key``. +* **Model ids** — must carry the ``anthropic.`` prefix (``anthropic.claude-opus-5``). + A bare id or a cross-region ``us.anthropic.`` id returns 404 here. +* **No structured outputs** — ``output_config.format`` and per-tool ``strict`` + are rejected outright ("Extra inputs are not permitted"), so schema requests + fall back to the inherited prompt-based JSON path. +""" + +import logging +import os +from typing import Dict, Optional + +from marsys.models.adapters.anthropic import AnthropicAdapter, AsyncAnthropicAdapter + +logger = logging.getLogger(__name__) + +DEFAULT_BEDROCK_REGION = "us-east-1" + + +def bedrock_base_url(region: Optional[str] = None) -> str: + """Messages-API-shaped Bedrock base URL for a region. + + Region resolution order: explicit argument, ``AWS_REGION``, + ``AWS_DEFAULT_REGION``, then ``us-east-1``. + """ + resolved = ( + region + or os.getenv("AWS_REGION") + or os.getenv("AWS_DEFAULT_REGION") + or DEFAULT_BEDROCK_REGION + ) + return f"https://bedrock-mantle.{resolved}.api.aws/anthropic/v1" + + +def normalize_bedrock_model_id(model_name: str) -> str: + """Return the id spelling this endpoint accepts. + + Bedrock requires exactly one ``anthropic.`` prefix. Callers legitimately + arrive with any of three spellings — a bare id from the shared catalog, an + ``anthropic/``-prefixed id copied from OpenRouter, or a cross-region + ``us.anthropic.`` id copied from the AWS console — and the two wrong ones + 404. Normalizing here keeps the caller's id portable. + """ + name = (model_name or "").strip() + if not name: + return name + for prefix in ("us.anthropic.", "eu.anthropic.", "apac.anthropic.", "anthropic/"): + if name.lower().startswith(prefix): + name = name[len(prefix):] + break + if not name.lower().startswith("anthropic."): + name = f"anthropic.{name}" + return name + + +class BedrockAdapter(AnthropicAdapter): + """Claude on Amazon Bedrock via the Messages-API-shaped endpoint.""" + + # The endpoint rejects `output_config.format` and per-tool `strict`; the + # inherited prompt-based JSON fallback carries the schema instead. + supports_structured_output = False + + def __init__( + self, + model_name: str, + api_key: str = "", + base_url: str = "", + max_tokens: int = 1024, + temperature: float = 0.7, + region: Optional[str] = None, + **kwargs, + ): + super().__init__( + model_name=normalize_bedrock_model_id(model_name), + api_key=api_key or os.getenv("AWS_BEARER_TOKEN_BEDROCK", ""), + base_url=base_url or bedrock_base_url(region), + max_tokens=max_tokens, + temperature=temperature, + **kwargs, + ) + + def get_headers(self) -> Dict[str, str]: + return { + "Authorization": f"Bearer {self.api_key}", + "Content-Type": "application/json", + "anthropic-version": "2023-06-01", + } + + def _provider_name(self) -> Optional[str]: + # The base implementation derives this from the class name; keep it + # pinned so retry/error settings resolve under the provider id the rest + # of the stack uses. + return "bedrock" + + def report_model_id(self, echoed: Optional[str]) -> str: + """Report the ``anthropic.``-prefixed id, not Bedrock's bare echo. + + A request for ``anthropic.claude-sonnet-5`` comes back as + ``claude-sonnet-5``, and ``anthropic.claude-haiku-4-5`` as + ``claude-haiku-4-5-20251001`` — ids that exist in the *first-party* + namespace, so a cost table keyed by Bedrock ids finds no rate and + silently prices the call at zero. Prefer the echo only when it round-trips + to the same id we asked for, so a genuine snapshot resolution is still + visible. + """ + if echoed and normalize_bedrock_model_id(echoed) == self.model_name: + return normalize_bedrock_model_id(echoed) + return self.model_name + + +class AsyncBedrockAdapter(AsyncAnthropicAdapter, BedrockAdapter): + """Async Bedrock adapter. + + Inherits SSE streaming from :class:`AsyncAnthropicAdapter` (the endpoint + returns ``text/event-stream``) and the auth/id/capability deltas from + :class:`BedrockAdapter`. + """ + + def _provider_name(self) -> Optional[str]: + return "bedrock" diff --git a/src/marsys/models/adapters/factory.py b/src/marsys/models/adapters/factory.py index 6f488b37..d2eb7f19 100644 --- a/src/marsys/models/adapters/factory.py +++ b/src/marsys/models/adapters/factory.py @@ -4,6 +4,7 @@ from marsys.models.adapters.openai import OpenAIAdapter from marsys.models.adapters.openrouter import OpenRouterAdapter from marsys.models.adapters.anthropic import AnthropicAdapter +from marsys.models.adapters.bedrock import BedrockAdapter from marsys.models.adapters.google import GoogleAdapter from marsys.models.adapters.openai_oauth import OpenAIOAuthAdapter from marsys.models.adapters.anthropic_oauth import AnthropicOAuthAdapter @@ -25,6 +26,7 @@ def create_adapter( adapters = { "openai": OpenAIAdapter, "anthropic": AnthropicAdapter, + "bedrock": BedrockAdapter, # Claude on Amazon Bedrock (Messages-API-shaped) "google": GoogleAdapter, "openrouter": OpenRouterAdapter, # OpenRouter with additional headers support "xai": OpenRouterAdapter, # xAI Grok uses OpenAI-compatible /chat/completions diff --git a/src/marsys/models/adapters/google.py b/src/marsys/models/adapters/google.py index f630059b..7619f869 100644 --- a/src/marsys/models/adapters/google.py +++ b/src/marsys/models/adapters/google.py @@ -244,6 +244,14 @@ def format_request_payload(self, messages: List[Dict], **kwargs) -> Dict[str, An # Add native function calling support if kwargs.get("tools"): + if any(isinstance(t, dict) and t.get("defer_loading") for t in kwargs["tools"]): + # Google has no deferred-tool-loading feature; the rebuild below already drops the + # per-tool defer_loading flag, so warn (not a silent drop) and fall back to eager. + import warnings + warnings.warn( + "Google models do not support deferred tool loading; the per-tool " + "defer_loading flag is ignored and all tools are loaded eagerly." + ) # Convert OpenAI format tools to Google format google_tools = [] for tool in kwargs["tools"]: diff --git a/src/marsys/models/adapters/openai.py b/src/marsys/models/adapters/openai.py index b5c080c2..45c17fbc 100644 --- a/src/marsys/models/adapters/openai.py +++ b/src/marsys/models/adapters/openai.py @@ -225,27 +225,44 @@ def convert_content_types(content): payload["text"] = {"format": {"type": "json_object"}} # Handle tools - Responses API uses flattened structure (internally tagged) - # Converts externally tagged format to internally tagged format + # Converts externally tagged format to internally tagged format. + # A per-tool ``defer_loading: true`` rides the Chat-Completions tool dict top-level + # (deferred tool loading); it maps onto the flat Responses tool and triggers the + # ``tool_search`` built-in so deferred tools are discovered on demand (their schemas stay + # out of the cached prefix). Nothing deferred → byte-identical to before. if kwargs.get("tools"): tools = kwargs["tools"] converted_tools = [] + any_deferred = False for tool in tools: if isinstance(tool, dict): if tool.get("type") == "function" and "function" in tool: # Convert from Chat Completions format (externally tagged) func = tool["function"] - converted_tools.append({ + converted = { "type": "function", "name": func.get("name"), "description": func.get("description"), "parameters": func.get("parameters"), # Note: strict is true by default in Responses API - }) + } + if tool.get("defer_loading"): + converted["defer_loading"] = True + any_deferred = True + converted_tools.append(converted) else: # Already in Responses API format or other tool type converted_tools.append(tool) + if isinstance(tool, dict) and tool.get("defer_loading"): + any_deferred = True else: converted_tools.append(tool) + if any_deferred and not any( + isinstance(t, dict) and t.get("type") == "tool_search" for t in converted_tools + ): + # Auto-add the Responses tool-search built-in so deferred tools are discoverable + # (gpt-5.4+). Suppressed if the caller supplied their own. + converted_tools.append({"type": "tool_search"}) payload["tools"] = converted_tools # Handle OpenAI reasoning (effort-based for all models via Responses API) diff --git a/src/marsys/models/adapters/openai_oauth.py b/src/marsys/models/adapters/openai_oauth.py index 12b38bfb..d2fb34c7 100644 --- a/src/marsys/models/adapters/openai_oauth.py +++ b/src/marsys/models/adapters/openai_oauth.py @@ -320,19 +320,33 @@ def format_request_payload( """Format request payload for ChatGPT backend.""" instructions, input_messages = self._convert_messages_to_chatgpt_format(messages) - # Convert tools to Responses API format + # Convert tools to Responses API format. A per-tool ``defer_loading: true`` rides the tool + # dict top-level (deferred tool loading); it maps onto the flat Responses tool and triggers + # the ``tool_search`` built-in so deferred tools are discovered on demand. Nothing deferred + # → byte-identical to before. tools_list = [] + any_deferred = False if kwargs.get("tools"): for t in kwargs["tools"]: if t.get("type") == "function": - tools_list.append({ + converted = { "type": "function", "name": t["function"]["name"], "description": t["function"].get("description", ""), "parameters": t["function"].get("parameters", {}) - }) + } + if t.get("defer_loading"): + converted["defer_loading"] = True + any_deferred = True + tools_list.append(converted) else: tools_list.append(t) + if isinstance(t, dict) and t.get("defer_loading"): + any_deferred = True + if any_deferred and not any( + isinstance(t, dict) and t.get("type") == "tool_search" for t in tools_list + ): + tools_list.append({"type": "tool_search"}) # Build payload - all required fields for ChatGPT backend # Note: max_output_tokens is NOT supported by ChatGPT backend diff --git a/src/marsys/models/adapters/openrouter.py b/src/marsys/models/adapters/openrouter.py index 2eb36d7b..2801ea36 100644 --- a/src/marsys/models/adapters/openrouter.py +++ b/src/marsys/models/adapters/openrouter.py @@ -4,6 +4,7 @@ import warnings from typing import Any, Dict, List, Optional +from marsys.models.adapters.anthropic import _anthropic_model_rejects_temperature from marsys.models.adapters.base import APIProviderAdapter, AsyncBaseAPIAdapter from marsys.models.response_models import ( ErrorResponse, @@ -114,10 +115,18 @@ def format_request_payload(self, messages: List[Dict], **kwargs) -> Dict[str, An # Temperature must NOT use `or`: an explicit 0.0 is a valid value and # must survive to the wire. None-gate instead. + # + # Claude's reasoning-capable line rejects the parameter outright, and the + # gateway forwards it, so an `anthropic/claude-opus-5`-style route needs + # the same omission the direct Anthropic adapter applies. Reuses that + # adapter's predicate (it strips the `anthropic/` prefix itself) so one + # capability table governs the model wherever it is reached from. temperature = kwargs.get("temperature") if temperature is None: temperature = self.temperature - if temperature is not None: + if temperature is not None and not _anthropic_model_rejects_temperature( + self.model_name + ): payload["temperature"] = temperature if kwargs.get("top_p") is not None: @@ -176,7 +185,22 @@ def format_request_payload(self, messages: List[Dict], **kwargs) -> Dict[str, An payload["response_format"] = {"type": "json_object"} if kwargs.get("tools"): - payload["tools"] = kwargs["tools"] + tools = kwargs["tools"] + if any(isinstance(t, dict) and t.get("defer_loading") for t in tools): + # OpenRouter has no deferred-tool-loading feature and forwards `tools` verbatim, + # so a defer_loading marker would reach the wire (possible 400). Strip it and fall + # back to eager loading. Warn — a SILENT behavior change is what the additive + # contract forbids. + import warnings + warnings.warn( + "OpenRouter does not support deferred tool loading; the per-tool defer_loading " + "flag is stripped and all tools are loaded eagerly." + ) + tools = [ + {k: v for k, v in t.items() if k != "defer_loading"} if isinstance(t, dict) else t + for t in tools + ] + payload["tools"] = tools # Handle OpenRouter-specific reasoning configuration # Import model detection utility diff --git a/src/marsys/models/models.py b/src/marsys/models/models.py index a66f9f70..413598a6 100644 --- a/src/marsys/models/models.py +++ b/src/marsys/models/models.py @@ -47,6 +47,9 @@ # Anthropic AnthropicAdapter, AsyncAnthropicAdapter, + # Bedrock — resolved by name for the async twin, so it must be in scope here + BedrockAdapter, + AsyncBedrockAdapter, # Google GoogleAdapter, AsyncGoogleAdapter, @@ -70,6 +73,16 @@ # --- Model Configuration Schema --- + +def _bedrock_default_base_url() -> str: + """Bedrock's base URL carries the AWS region, so it cannot be a fixed + literal like the other providers'. Imported lazily to keep this module + free of an adapter-module import at definition time.""" + from marsys.models.adapters.bedrock import bedrock_base_url + + return bedrock_base_url() + + # Define the provider base URLs dictionary PROVIDER_BASE_URLS = { "openai": "https://api.openai.com/v1/", @@ -77,6 +90,10 @@ "google": "https://generativelanguage.googleapis.com/v1beta", # Gemini API base URL "anthropic": "https://api.anthropic.com/v1", "xai": "https://api.x.ai/v1", # xAI Grok API (OpenAI-compatible) + # Claude on Amazon Bedrock, Messages-API-shaped endpoint. Region-dependent, + # so this entry is the AWS_REGION-resolved default; the adapter re-resolves + # it per instance (see adapters/bedrock.bedrock_base_url). + "bedrock": _bedrock_default_base_url(), "openai-oauth": "https://chatgpt.com/backend-api/codex/responses", # ChatGPT OAuth endpoint "anthropic-oauth": "https://api.anthropic.com/v1/messages?beta=true", # Claude OAuth endpoint } @@ -98,7 +115,7 @@ class ModelConfig(BaseModel): description="Model identifier (e.g., 'gpt-4o', 'mistralai/Mistral-7B-Instruct-v0.1')", ) provider: Optional[ - Literal["openai", "openrouter", "google", "anthropic", "xai", "openai-oauth", "anthropic-oauth"] + Literal["openai", "openrouter", "google", "anthropic", "xai", "bedrock", "openai-oauth", "anthropic-oauth"] ] = Field( None, description="API provider name (used to determine base_url if not set)" ) @@ -221,6 +238,9 @@ def _validate_api_key(self) -> "ModelConfig": "google": "GOOGLE_API_KEY", "anthropic": "ANTHROPIC_API_KEY", "xai": "XAI_API_KEY", + # Bedrock authenticates with a bearer token, not SigV4, on the + # Messages-API-shaped endpoint this stack targets. + "bedrock": "AWS_BEARER_TOKEN_BEDROCK", } # Providers that use OAuth or other credential mechanisms (not API keys) oauth_providers = {"openai-oauth", "anthropic-oauth"} @@ -288,7 +308,10 @@ def _validate_thinking_config(self) -> "ModelConfig": # Validate reasoning_effort values if self.reasoning_effort is not None: - valid_efforts = ["minimal", "low", "medium", "high"] + # "xhigh"/"max" are the upper tiers the reasoning-capable Claude + # models accept on `output_config.effort`; without them here the + # config layer rejects a value the provider supports. + valid_efforts = ["minimal", "low", "medium", "high", "xhigh", "max"] if self.reasoning_effort.lower() not in valid_efforts: raise ValueError( f"Invalid reasoning_effort '{self.reasoning_effort}'. " @@ -742,7 +765,13 @@ async def arun( max_tokens: Overrides the default max_tokens for this specific call. temperature: Overrides the default temperature for this specific call. top_p: Overrides the default top_p for this specific call. - tools: Optional list of tools for function calling. + tools: Optional list of tools for function calling. A per-tool ``defer_loading: true`` + flag on a tool dict marks it for DEFERRED loading: the provider's tool-search + built-in discovers it on demand and the definition rides the message tail, so its + schema stays out of the billed/cached prefix (the prompt cache survives a load). + Honored on Anthropic (api-key + OAuth) and OpenAI Responses (api-key + OAuth); + stripped on OpenRouter and warned-then-ignored on Google (no provider feature). + No signature change — the flag rides the existing ``tools`` array. Default: all eager. **kwargs: Additional parameters to pass to the API. Returns: diff --git a/src/marsys/models/response_models.py b/src/marsys/models/response_models.py index 92a6ee37..6e17b882 100644 --- a/src/marsys/models/response_models.py +++ b/src/marsys/models/response_models.py @@ -27,15 +27,36 @@ def validate_function(cls, v): class UsageInfo(BaseModel): - """Token usage information.""" + """Token usage information. + + ``prompt_tokens`` is the provider's *uncached* prompt count. With prompt + caching active it is NOT the whole prompt: the full prompt is + ``prompt_tokens + cache_creation_input_tokens + cache_read_input_tokens`` + (Anthropic's own definition). A consumer that sizes a conversation, prices a + call, or bounds prompt growth must read ``full_prompt_tokens`` — reading + ``prompt_tokens`` alone silently under-measures by up to ~10x once a cached + prefix exists. Both cache fields are None on providers that report no cache + figures, which is why ``full_prompt_tokens`` coalesces rather than sums + blindly. + """ prompt_tokens: Optional[int] = None completion_tokens: Optional[int] = None total_tokens: Optional[int] = None reasoning_tokens: Optional[int] = None # For o1 models - + # Prompt-cache accounting. Populated by the providers that report it + # (Anthropic api-key, Bedrock, Anthropic OAuth); None everywhere else. + cache_read_input_tokens: Optional[int] = None + cache_creation_input_tokens: Optional[int] = None + @model_validator(mode='after') def calculate_total(self): - """Calculate total tokens if not provided.""" + """Calculate total tokens if not provided. + + Deliberately unchanged by the cache fields: ``total_tokens`` keeps its + established meaning (prompt + completion + reasoning) so a response that + reports no cache figures harmonizes to the same number it always did. + The cache-aware reading is ``full_prompt_tokens``. + """ if self.total_tokens is None: prompt = self.prompt_tokens or 0 completion = self.completion_tokens or 0 @@ -43,6 +64,20 @@ def calculate_total(self): self.total_tokens = prompt + completion + reasoning return self + @property + def full_prompt_tokens(self) -> int: + """The whole prompt the provider processed, cached parts included. + + The number a context bound, a spend ledger, and a runaway-growth backstop + all actually want. Equals ``prompt_tokens`` exactly when no cache was + involved, so it is a safe unconditional substitute at every such site. + """ + return ( + (self.prompt_tokens or 0) + + (self.cache_creation_input_tokens or 0) + + (self.cache_read_input_tokens or 0) + ) + class ResponseMetadata(BaseModel): """Metadata about the API response.""" diff --git a/src/marsys/models/serialize.py b/src/marsys/models/serialize.py index bed09605..1f434683 100644 --- a/src/marsys/models/serialize.py +++ b/src/marsys/models/serialize.py @@ -38,6 +38,7 @@ "google", "anthropic", "xai", + "bedrock", "openai-oauth", "anthropic-oauth", ] diff --git a/tests/agents/test_exceptions.py b/tests/agents/test_exceptions.py index 9ba5d8fe..fb73344b 100644 --- a/tests/agents/test_exceptions.py +++ b/tests/agents/test_exceptions.py @@ -401,6 +401,88 @@ def test_model_api_error(self): assert error.retry_after == 60 +class TestTransportErrorClassification: + """A transport-layer failure (connect/DNS/timeout/reset) is raised by the HTTP + client with NO status code and NO provider error body. It must classify as a + transient, retryable NETWORK_ERROR/TIMEOUT — not the UNKNOWN, non-retryable + default that would terminally kill an otherwise-recoverable turn. Real regression: + a machine-wide DNS blip raised httpx.ConnectError('[Errno 11001] getaddrinfo + failed') and the turn was permanently dropped instead of retried.""" + + def test_httpx_connect_error_is_retryable_network(self): + import httpx + + err = ModelAPIError.from_provider_response( + provider="anthropic-oauth", + exception=httpx.ConnectError("[Errno 11001] getaddrinfo failed"), + ) + assert err.classification == "network_error" + assert err.is_retryable is True + + def test_httpcore_connect_error_is_retryable_network(self): + import httpcore + + err = ModelAPIError.from_provider_response( + provider="anthropic-oauth", + exception=httpcore.ConnectError("getaddrinfo failed"), + ) + assert err.classification == "network_error" + assert err.is_retryable is True + + def test_httpx_read_timeout_is_retryable_timeout(self): + import httpx + + err = ModelAPIError.from_provider_response( + provider="anthropic-oauth", + exception=httpx.ReadTimeout("The read operation timed out"), + ) + assert err.classification == "timeout" + assert err.is_retryable is True + + def test_stdlib_connection_reset_is_retryable_network(self): + """No httpx/httpcore/aiohttp in the MRO — a bare stdlib ConnectionError + (e.g. a mid-stream RST) still classifies via the name table.""" + err = ModelAPIError.from_provider_response( + provider="anthropic-oauth", + exception=ConnectionResetError("Connection reset by peer"), + ) + assert err.classification == "network_error" + assert err.is_retryable is True + + def test_local_protocol_error_stays_non_retryable(self): + """An our-side/config transport fault (bad request framing, unsupported + protocol) shares the transport tree but is NOT transient — it must keep the + UNKNOWN, non-retryable default so we don't hammer a doomed request.""" + import httpx + + err = ModelAPIError.from_provider_response( + provider="anthropic-oauth", + exception=httpx.LocalProtocolError("bad chunk"), + ) + assert err.classification == "unknown" + assert err.is_retryable is False + + def test_status_code_wins_over_transport_fallback(self): + """Precedence guard: when a real HTTP status was parsed, the transport + fallback must NOT fire — a 401 stays AUTHENTICATION_FAILED even if the + carrying exception's name is in the transport table.""" + + class FakeResp: + status_code = 401 + + def json(self): + return {} + + err = ModelAPIError.from_provider_response( + provider="anthropic-oauth", + response=FakeResp(), + exception=ConnectionError("noise"), + ) + assert err.status_code == 401 + assert err.classification == "authentication_failed" + assert err.is_retryable is False + + # ============================================================================= # Browser Error Tests # ============================================================================= diff --git a/tests/agents/test_serialize.py b/tests/agents/test_serialize.py index 7598aa69..60849c2f 100644 --- a/tests/agents/test_serialize.py +++ b/tests/agents/test_serialize.py @@ -444,3 +444,73 @@ def test_agent_round_trip_preserves_identity_fields(): assert re.instruction == original.instruction assert re.max_tokens == original.max_tokens assert "Peer1" in re._allowed_peers_init + + +# --------------------------------------------------------------------------- +# can_escalate (ADR-013 wire-mirror completion): the escalate_to_user grant +# round-trips through AgentSpec, mirroring bidirectional_peers. +# --------------------------------------------------------------------------- + + +def test_can_escalate_true_round_trip_via_spec(): + """AC-1: a spec carrying can_escalate=True hydrates to a live Agent granted it.""" + spec = WorkflowDefinition( + topology=TopologySpec( + nodes=[NodeSpec(name="EscT", agent_ref="EscT")], edges=[] + ), + agents={ + "EscT": AgentSpec( + name="EscT", + goal="g", + instruction="i", + model=_model_config_spec(), + can_escalate=True, + ), + }, + ) + agents = asyncio.run(pydantic_to_agents(spec, tool_registry={})) + assert agents[0].can_escalate is True + + +def test_can_escalate_default_false_backward_compat(): + """AC-1b: a serialized spec predating can_escalate hydrates to False (the + optional field's default) and validates despite AgentSpec's extra='forbid'.""" + legacy = AgentSpec( + name="EscLegacy", goal="g", instruction="i", model=_model_config_spec() + ).model_dump() + legacy.pop("can_escalate", None) # simulate at-rest JSON predating the field + spec = AgentSpec.model_validate(legacy) + assert spec.can_escalate is False + workflow = WorkflowDefinition( + topology=TopologySpec( + nodes=[NodeSpec(name="EscLegacy", agent_ref="EscLegacy")], edges=[] + ), + agents={"EscLegacy": spec}, + ) + agents = asyncio.run(pydantic_to_agents(workflow, tool_registry={})) + assert agents[0].can_escalate is False + + +def test_can_escalate_live_agent_round_trip(): + """AC-1: live Agent(can_escalate=True) -> AgentSpec -> JSON -> AgentSpec -> Agent.""" + original = Agent( + name="EscRT", + goal="g", + instruction="i", + model_config=_model_config(), + can_escalate=True, + ) + spec = agent_to_pydantic(original) + assert spec.can_escalate is True + rehydrated_spec = AgentSpec.model_validate_json(spec.model_dump_json()) + assert rehydrated_spec.can_escalate is True + + AgentRegistry.unregister(original.name) + workflow = WorkflowDefinition( + topology=TopologySpec( + nodes=[NodeSpec(name="EscRT", agent_ref="EscRT")], edges=[] + ), + agents={"EscRT": rehydrated_spec}, + ) + agents = asyncio.run(pydantic_to_agents(workflow, tool_registry={})) + assert agents[0].can_escalate is True diff --git a/tests/coordination/state/test_snapshot.py b/tests/coordination/state/test_snapshot.py index 2e96cb96..890e3162 100644 --- a/tests/coordination/state/test_snapshot.py +++ b/tests/coordination/state/test_snapshot.py @@ -97,6 +97,7 @@ def test_state_snapshot_top_level_field_set_is_stable(): "completed_emitted", "user_interactions", "user_interaction_inflight", + "pending_user_interaction", # ADR-012: the in-flight durable interaction "max_steps", } assert set(schema["properties"].keys()) == expected_fields @@ -215,15 +216,17 @@ def test_state_snapshot_with_non_json_arrived_value_raises_on_dump(): def test_user_interaction_state_shape(): - """AC-10: UserInteractionState carries the four documented fields.""" + """AC-10: UserInteractionState carries the documented fields (+ ADR-012's + durable bit, which defaults False).""" ui = UserInteractionState( suspended_branch_id="br_0003", prompt="please confirm", resume_agent="Coordinator", delivery_target="bar_0000", ) - expected = {"suspended_branch_id", "prompt", "resume_agent", "delivery_target"} + expected = {"suspended_branch_id", "prompt", "resume_agent", "delivery_target", "durable"} assert set(ui.model_dump().keys()) == expected + assert ui.durable is False # ADR-012: default keeps pre-feature snapshots valid def test_paused_session_metadata_shape(): diff --git a/tests/coordination/test_durable_hitl.py b/tests/coordination/test_durable_hitl.py new file mode 100644 index 00000000..f60ab3db --- /dev/null +++ b/tests/coordination/test_durable_hitl.py @@ -0,0 +1,438 @@ +"""Durable human-in-the-loop suspend/resume — the orchestrator mechanism (ADR-012). + +Deterministic coverage of the durable `ask_user`/`UserNode` path, driven through +the LIVE `Orchestrator` via `DeterministicRuntime` (the same pattern as +`tests/integration/test_pause_resume.py`, NOT the drifted simulator). These tests +exercise the mechanism without a model call: + + - durable `enqueue_user_interaction` records the in-flight interaction in the + `pending_user_interaction` scalar and spawns NO `_drive` task (AC-1/AC-4); + - the dispatch loop snapshots-and-exits with the `awaiting_user` sentinel + rather than blocking on the in-memory queue (AC-3); + - `snapshot()` captures the four in-flight fields and round-trips through + `StateSnapshot` (AC-5/7/8/9/10), with old snapshots defaulting cleanly (AC-11); + - restore → inject (via the existing `resume_branch_with_user_response` seam, + which clears the scalar) → resume drives the resume_agent to terminal with + the response as its input (AC-12/13/14); + - a resumed run can re-suspend at a second durable interaction (AC-19); + - `Orchestra.resume_session`'s argument contract (AC-16/17/18) and version lock + (AC-26); and the workflow-definition `durable` plumbing through the shim + (AC-24/25 — the spec → durable UserNode path). + +The full `Orchestra.execute()` round-trip with real agents (and cost-across-resume) +is the gated live test in `tests/integration/test_durable_hitl_live.py`. +""" +from __future__ import annotations + +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +import pytest + +import marsys +from marsys.coordination.execution.deterministic_runtime import DeterministicRuntime +from marsys.coordination.execution.det_nodes import UserNode +from marsys.coordination.execution.orchestrator import Orchestrator +from marsys.coordination.execution.orchestrator_types import ( + ConvergencePolicy, + StepResult, + reset_ids, +) +from marsys.coordination.orchestra import Orchestra +from marsys.coordination.state import ( + FileStorageBackend, + IncompatibleSnapshotError, + StateSnapshot, +) +from marsys.coordination.state.snapshot import UserInteractionState +from marsys.coordination.topology.core import Node, NodeKind, Topology + +from tests.coordination.orchestrator._helpers import build_topology + + +# ─── Helpers ───────────────────────────────────────────────────────────────── + +# A durable interaction never invokes the handler (it bypasses `_drive`); the +# UserNode only needs a non-None handler to pass on_single_invoke's guard. +_DUMMY_HANDLER = object() + + +def _durable_topology(): + """Start → A → User(durable) → B → End. A SINGLE_INVOKEs User; the resume + agent (User's successor) is B.""" + return build_topology( + nodes=[ + "Start", "A", + UserNode("User", handler=_DUMMY_HANDLER, durable=True), + "B", "End", + ], + flows=["Start -> A", "A -> User", "User -> B", "B -> End"], + ) + + +def _suspend_at_durable_user(): + """Run a fresh durable workload to its awaiting-user suspend and return + (orchestrator, result). A is scripted to SINGLE_INVOKE the durable User + node; the run snapshots-and-exits there.""" + reset_ids() + topo = _durable_topology() + runtime = DeterministicRuntime() + runtime.queue_agent("A", StepResult( + kind="SINGLE_INVOKE", next_agent="User", value="please authenticate", + )) + orch = Orchestrator(topo, runtime, ConvergencePolicy()) + return orch, topo, runtime + + +def _make_orchestra(tmp_path: Path) -> Orchestra: + return Orchestra( + agent_registry=type("R", (), {"get": staticmethod(lambda n: None), + "clear": staticmethod(lambda: None)}), + storage_backend=FileStorageBackend(tmp_path / "snapshots"), + ) + + +async def _write_snapshot(orch: Orchestra, sid: str, snapshot: StateSnapshot) -> None: + await orch.storage_backend.write( + f"{sid}/snapshot.json", snapshot.model_dump_json().encode("utf-8") + ) + + +def _durable_snapshot(sid: str, *, version: str | None = None, + pending: UserInteractionState | None) -> StateSnapshot: + now = datetime.now(tz=timezone.utc) + return StateSnapshot( + framework_version=version or marsys.__version__, + session_id=sid, + topology_digest="x", + created_at=now, + paused_at=now, + branches={}, barriers={}, convergence_barriers={}, + runnable=[], fire_queue=[], completed_emitted=[], + user_interactions=[], user_interaction_inflight=pending is not None, + pending_user_interaction=pending, + ) + + +# ─── Durable enqueue + dispatch-loop exit ──────────────────────────────────── + + +@pytest.mark.asyncio +async def test_durable_enqueue_sets_scalar_and_spawns_no_drive(): + """AC-1 + AC-4: reaching a durable user interaction records the in-flight + interaction in pending_user_interaction and spawns NO in-memory `_drive` + (the resume queue is never created).""" + orch, _topo, _rt = _suspend_at_durable_user() + result = await orch.run(task="go", entry_agent="A") + + assert orch.pending_user_interaction is not None # AC-1 + bid, prompt, resume_agent, target, durable = orch.pending_user_interaction + assert durable is True # the durable bit is carried + assert prompt == "please authenticate" + assert resume_agent == "B" # User's successor + assert orch._resume_user_responses is None # AC-4: no _drive/queue + assert result.error == "awaiting_user" # AC-3 sentinel + + +@pytest.mark.asyncio +async def test_durable_run_returns_promptly_without_blocking(): + """AC-21 (no-hang): a durably-parked run does NOT block on an in-memory + queue — run() returns the awaiting-user result rather than waiting for a + human (provable by completing well within a tight timeout).""" + import asyncio + orch, _topo, _rt = _suspend_at_durable_user() + result = await asyncio.wait_for(orch.run(task="go", entry_agent="A"), timeout=5.0) + assert result.error == "awaiting_user" + + +# ─── Snapshot capture + round-trip ─────────────────────────────────────────── + + +@pytest.mark.asyncio +async def test_snapshot_captures_pending_and_round_trips(tmp_path): + """AC-5/7/8/9/10: snapshot() captures the four in-flight fields, and the + OrchestratorState ↔ StateSnapshot mapping round-trips them through JSON.""" + orch, _topo, _rt = _suspend_at_durable_user() + await orch.run(task="go", entry_agent="A") + + state = orch.snapshot() + assert state.pending_user_interaction is not None + bid, prompt, resume_agent, target, _durable = state.pending_user_interaction + + # OrchestratorState → StateSnapshot (wire) via the Orchestra mapper (the path + # pause_session / _snapshot_and_write use), then JSON → back. + helper = _make_orchestra(tmp_path) + helper.canonical_topology = type("T", (), {"nodes": [], "edges": []})() + snap = helper._build_state_snapshot("sid", orch) + + assert snap.pending_user_interaction is not None + assert snap.pending_user_interaction.suspended_branch_id == bid # AC-7 + assert snap.pending_user_interaction.prompt == prompt # AC-8 + assert snap.pending_user_interaction.resume_agent == resume_agent # AC-9 + assert snap.pending_user_interaction.delivery_target == target # AC-10 + + # JSON round-trip → back to OrchestratorState preserves the 4-tuple. + reloaded = StateSnapshot.model_validate_json(snap.model_dump_json()) + back = helper._snapshot_to_orchestrator_state(reloaded) + assert back.pending_user_interaction == state.pending_user_interaction + + +def test_old_snapshot_without_pending_defaults_clean(): + """AC-11: a snapshot written before this feature (no pending field) loads + and defaults pending_user_interaction to None under extra='forbid'.""" + now = datetime.now(tz=timezone.utc) + raw = { + "framework_version": marsys.__version__, "session_id": "old", + "topology_digest": "x", "created_at": now.isoformat(), + "paused_at": now.isoformat(), "branches": {}, "barriers": {}, + "convergence_barriers": {}, "runnable": [], "fire_queue": [], + "completed_emitted": [], "user_interactions": [], + "user_interaction_inflight": False, + } + import json + snap = StateSnapshot.model_validate_json(json.dumps(raw)) + assert snap.pending_user_interaction is None + + +# ─── Restore → inject → resume to terminal ─────────────────────────────────── + + +@pytest.mark.asyncio +async def test_full_durable_round_trip_to_terminal(): + """AC-12/13/14: restore a durable suspend into a fresh Orchestrator, inject + the human's response via the existing seam, and drive the resume_agent to a + terminal success — with the response delivered as the resume_agent's input.""" + orch1, topo, _rt1 = _suspend_at_durable_user() + await orch1.run(task="go", entry_agent="A") + state = orch1.snapshot() + bid, _prompt, resume_agent, _target, _durable = state.pending_user_interaction + + # Fresh Orchestrator (simulating a restart), restore, inject, resume. + rt2 = DeterministicRuntime() + rt2.queue_agent("B", StepResult(kind="FINAL_RESPONSE", value="farewell")) + orch2 = Orchestrator(_durable_topology(), rt2, ConvergencePolicy()) + orch2.restore_from(state) + assert orch2.pending_user_interaction is not None # restored + + orch2.resume_branch_with_user_response(bid, "AUTH_DONE", resume_agent) + assert orch2.pending_user_interaction is None # cleared on consume + + # AC-13: the resume_agent branch carries the response as its input. + b_inputs = [br.input for br in orch2.branches.values() if br.current_agent == "B"] + assert "AUTH_DONE" in b_inputs + + result = await orch2.resume() + assert result.success # AC-12/14 terminal + + +@pytest.mark.asyncio +async def test_resumed_run_can_resuspend_at_second_durable_interaction(): + """AC-19: a resumed run that reaches a SECOND durable interaction durably + suspends again (the awaiting-user exit is reachable from resume(), not only + the first run()).""" + reset_ids() + topo = build_topology( + nodes=[ + "Start", "A", + UserNode("User", handler=_DUMMY_HANDLER, durable=True), + "B", + UserNode("User2", handler=_DUMMY_HANDLER, durable=True), + "C", "End", + ], + flows=[ + "Start -> A", "A -> User", "User -> B", "B -> User2", + "User2 -> C", "C -> End", + ], + ) + rt1 = DeterministicRuntime() + rt1.queue_agent("A", StepResult(kind="SINGLE_INVOKE", next_agent="User", value="q1")) + orch1 = Orchestrator(topo, rt1, ConvergencePolicy()) + r1 = await orch1.run(task="go", entry_agent="A") + assert r1.error == "awaiting_user" + state1 = orch1.snapshot() + bid1, _p1, ra1, _t1, _d1 = state1.pending_user_interaction + + # Resume: B runs, SINGLE_INVOKEs the SECOND durable User node → re-suspends. + rt2 = DeterministicRuntime() + rt2.queue_agent("B", StepResult(kind="SINGLE_INVOKE", next_agent="User2", value="q2")) + orch2 = Orchestrator(topo, rt2, ConvergencePolicy()) + orch2.restore_from(state1) + orch2.resume_branch_with_user_response(bid1, "first-done", ra1) + r2 = await orch2.resume() + assert r2.error == "awaiting_user" # AC-19 re-suspend + assert orch2.pending_user_interaction is not None + assert orch2.pending_user_interaction[2] == "C" # User2's successor + + # AC-19: the re-suspended run is itself resumable to terminal. + state2 = orch2.snapshot() + bid2, _p2, ra2, _t2, _d2 = state2.pending_user_interaction + rt3 = DeterministicRuntime() + rt3.queue_agent("C", StepResult(kind="FINAL_RESPONSE", value="all-done")) + orch3 = Orchestrator(topo, rt3, ConvergencePolicy()) + orch3.restore_from(state2) + orch3.resume_branch_with_user_response(bid2, "second-answer", ra2) + r3 = await orch3.resume() + assert r3.success # second resume → terminal + + +# ─── Orchestra.resume_session argument contract + version lock ──────────────── + + +@pytest.mark.asyncio +async def test_resume_session_pending_without_response_raises(tmp_path): + """AC-17: resume_session WITHOUT user_response on a snapshot that HAS a + pending durable interaction raises (a human-wait needs the answer).""" + orch = _make_orchestra(tmp_path) + sid = "pending-no-resp" + pending = UserInteractionState( + suspended_branch_id="b1", prompt="auth?", resume_agent="B", + delivery_target="root", durable=True, + ) + await _write_snapshot(orch, sid, _durable_snapshot(sid, pending=pending)) + with pytest.raises(ValueError, match="awaiting a user response"): + await orch.resume_session(sid) + + +@pytest.mark.asyncio +async def test_resume_session_response_without_pending_raises(tmp_path): + """AC-16: resume_session WITH user_response on a snapshot that has NO pending + durable interaction raises (the response is not silently dropped).""" + orch = _make_orchestra(tmp_path) + sid = "resp-no-pending" + await _write_snapshot(orch, sid, _durable_snapshot(sid, pending=None)) + with pytest.raises(ValueError, match="no pending durable"): + await orch.resume_session(sid, user_response="surprise") + + +@pytest.mark.asyncio +async def test_resume_session_user_response_is_keyword_only(tmp_path): + """AC-18: user_response is keyword-only — a positional fourth arg is a + TypeError (the `*` gates it alongside FW17's params).""" + orch = _make_orchestra(tmp_path) + with pytest.raises(TypeError): + await orch.resume_session("sid", None, None, "positional-response") + + +@pytest.mark.asyncio +async def test_resume_session_version_lock_on_durable_snapshot(tmp_path): + """AC-26: a durable snapshot under a mismatched framework_version fails with + IncompatibleSnapshotError (the version lock is inherited, not weakened).""" + orch = _make_orchestra(tmp_path) + sid = "durable-badver" + pending = UserInteractionState( + suspended_branch_id="b1", prompt="auth?", resume_agent="B", + delivery_target="root", durable=True, + ) + await _write_snapshot( + orch, sid, _durable_snapshot(sid, version="0.0.0-nope", pending=pending), + ) + with pytest.raises(IncompatibleSnapshotError): + await orch.resume_session(sid, user_response="x") + + +# ─── Workflow-definition durable trigger (founder option 2) ────────────────── + + +def test_spec_durable_user_node_materializes_durable(tmp_path): + """AC-24/25 (plumbing): a USER node carrying metadata['durable'] in the + topology spec materializes a durable UserNode through the analyze+shim path + that Orchestra.execute()/resume_session use for a spec-declared durable user + step. Uses an explicit Start det-node so the analyzer takes the modern entry + path (no legacy entry_point detection).""" + from marsys.coordination.config import ExecutionConfig + from marsys.coordination.topology.core import Edge + canonical = Topology( + nodes=[ + Node(name="Start", kind=NodeKind.START), + Node(name="A", kind=NodeKind.AGENT), + Node(name="User", kind=NodeKind.USER, metadata={"durable": True}), + Node(name="B", kind=NodeKind.AGENT), + Node(name="End", kind=NodeKind.END), + ], + edges=[ + Edge(source="Start", target="A"), + Edge(source="A", target="User"), + Edge(source="User", target="B"), + Edge(source="B", target="End"), + ], + ) + orch = _make_orchestra(tmp_path) + orch._build_topology_graph(canonical, ExecutionConfig()) + + user_dets = [ + d for d in (orch.topology_graph.det_nodes or {}).values() + if isinstance(d, UserNode) + ] + assert user_dets, "shim did not register a UserNode for the spec USER node" + assert any(d.durable for d in user_dets), ( + "spec metadata['durable'] did not materialize a durable UserNode" + ) + + +def test_spec_user_node_without_durable_is_sync(): + """A spec USER node WITHOUT durable metadata materializes a non-durable + (SYNC) UserNode — the flag is opt-in.""" + node = UserNode("User") + assert node.durable is False + + +# ─── Directive-style trigger (FW18's escalate_to_user path) ────────────────── + + +def test_enqueue_user_interaction_durable_directly(): + """AC-23: the directive-style trigger — calling enqueue_user_interaction with + durable=True DIRECTLY (no topology User node, as FW18's escalate_to_user does) + records the in-flight durable interaction and spawns no in-memory wait.""" + reset_ids() + topo = build_topology(nodes=["Start", "A", "End"], flows=["Start -> A", "A -> End"]) + orch = Orchestrator(topo, DeterministicRuntime(), ConvergencePolicy()) + orch.init_workflow(task="go", entry_agent="A") + branch = next(iter(orch.branches.values())) + + orch.enqueue_user_interaction(branch, prompt="re-auth?", resume_agent="A", durable=True) + + assert orch.pending_user_interaction is not None + bid, prompt, resume_agent, _target, durable = orch.pending_user_interaction + assert (prompt, resume_agent, durable) == ("re-auth?", "A", True) + assert orch._resume_user_responses is None # no _drive/queue spawned + assert branch.status == "WAITING" + + +def test_durable_sibling_re_arms_durably_not_sync(): + """ADR-012 FIFO durability (carry-the-bit guard): when a durable in-flight + interaction is resolved and a DURABLE sibling was queued, the sibling re-arms + DURABLY (it becomes the new durable pending, no in-memory _drive) — it does not + silently revert to the SYNC path.""" + reset_ids() + topo = build_topology(nodes=["Start", "A", "B", "End"], flows=["Start -> A", "A -> B", "B -> End"]) + orch = Orchestrator(topo, DeterministicRuntime(), ConvergencePolicy()) + orch.init_workflow(task="go", entry_agent="A") + b1 = next(iter(orch.branches.values())) + root = orch.root_barrier_id + + # One durable interaction in-flight, plus a second DURABLE branch queued as a + # sibling (the multi-pending-durable case the deque must not downgrade). + orch.enqueue_user_interaction(b1, prompt="q1", resume_agent="B", durable=True) + b2 = orch._spawn(agent="B", input="x", delivery_target=root, parent_spawn=None) + orch._user_interactions.append((b2.id, "q2", "B", root, True)) + + # Resolve the in-flight one → the sibling is popped and re-dispatched. + orch.resume_branch_with_user_response(b1.id, "answer1", "B") + + assert orch.pending_user_interaction is not None + assert orch.pending_user_interaction[4] is True # sibling re-armed DURABLY + assert orch._resume_user_responses is None # NOT a SYNC _drive + + +def test_no_spren_imports_in_framework(): + """AC-30: the framework has zero Spren coupling — no `from spren` / `import + spren` anywhere under src/marsys (SP-018). Runs the grep the AC names.""" + root = Path(__file__).resolve().parents[2] / "src" / "marsys" + hits = [] + for path in root.rglob("*.py"): + for line in path.read_text(encoding="utf-8").splitlines(): + s = line.strip() + if s.startswith("from spren") or s.startswith("import spren"): + hits.append(f"{path}: {s}") + assert not hits, f"Spren coupling found in the framework: {hits}" diff --git a/tests/coordination/test_escalate_to_user.py b/tests/coordination/test_escalate_to_user.py new file mode 100644 index 00000000..101e4a67 --- /dev/null +++ b/tests/coordination/test_escalate_to_user.py @@ -0,0 +1,324 @@ +"""Unit / mechanism tests for the `escalate_to_user` control directive (ADR-013). + +`escalate_to_user` is `ask_user` with the gate AXIS swapped: gated on the per-agent +`can_escalate` grant (NOT a topology User-node edge), enforced at BOTH the schema +offer and validation. The durable suspend/resume itself is framework 16, exercised +end-to-end in tests/integration/test_escalate_to_user.py. +""" +from __future__ import annotations + +import pytest +from unittest.mock import MagicMock, Mock + +from marsys.coordination.formats.coordination_tools import ( + COORDINATION_TOOL_NAMES, + CoordinationToolSchemaBuilder, + is_coordination_tool, +) +from marsys.coordination.formats.context import ( + AgentContext, + CoordinationContext, + SystemPromptContext, +) +from marsys.coordination.validation.response_validator import ( + ActionType, + ValidationProcessor, +) +from marsys.coordination.validation.types import ValidationErrorCategory +from marsys.coordination.topology.graph import TopologyGraph + + +class _FakeAgent: + """Minimal agent for validator tests — carries name + the can_escalate grant.""" + + def __init__(self, name: str = "A", can_escalate: bool = False): + self.name = name + self.can_escalate = can_escalate + + +# ── Schema offer (gated on can_escalate_user) ────────────────────────────── + +class TestEscalateSchema: + def test_in_coordination_tool_names(self): + assert "escalate_to_user" in COORDINATION_TOOL_NAMES + assert is_coordination_tool("escalate_to_user") is True + + def test_offered_when_granted(self): + schemas = CoordinationToolSchemaBuilder.build_schemas( + next_agents=[], can_escalate_user=True + ) + assert "escalate_to_user" in [s["function"]["name"] for s in schemas] + + def test_absent_when_not_granted(self): + schemas = CoordinationToolSchemaBuilder.build_schemas( + next_agents=[], can_escalate_user=False + ) + assert "escalate_to_user" not in [s["function"]["name"] for s in schemas] + + def test_default_off(self): + # Not passing can_escalate_user at all → absent (default off; AC-4/AC-26). + schemas = CoordinationToolSchemaBuilder.build_schemas( + next_agents=["Helper"], can_terminate_workflow=True, can_ask_user=True + ) + assert "escalate_to_user" not in [s["function"]["name"] for s in schemas] + + def test_uses_prompt_param(self): + schemas = CoordinationToolSchemaBuilder.build_schemas( + next_agents=[], can_escalate_user=True + ) + s = next(x for x in schemas if x["function"]["name"] == "escalate_to_user") + params = s["function"]["parameters"] + assert "prompt" in params["properties"] + assert params["properties"]["prompt"]["type"] == "string" + assert params["required"] == ["prompt"] + + def test_independent_of_ask_user(self): + only_escalate = [ + s["function"]["name"] + for s in CoordinationToolSchemaBuilder.build_schemas( + next_agents=[], can_ask_user=False, can_escalate_user=True + ) + ] + assert "escalate_to_user" in only_escalate and "ask_user" not in only_escalate + only_ask = [ + s["function"]["name"] + for s in CoordinationToolSchemaBuilder.build_schemas( + next_agents=[], can_ask_user=True, can_escalate_user=False + ) + ] + assert "ask_user" in only_ask and "escalate_to_user" not in only_ask + + +# ── Type completeness ────────────────────────────────────────────────────── + +def test_actiontype_has_escalate_user(): + assert hasattr(ActionType, "ESCALATE_USER") + + +def test_stepkind_has_escalate_user(): + from marsys.coordination.execution.orchestrator_types import StepKind + + assert "ESCALATE_USER" in StepKind.__args__ + + +# ── Validation (gated on the per-agent can_escalate grant) ────────────────── + +@pytest.fixture +def validator(): + g = Mock(spec=TopologyGraph) + g.get_next_agents = MagicMock(return_value=["End"]) + g.has_edge_to_usernode = MagicMock(return_value=False) + return ValidationProcessor(g) + + +class TestEscalateValidation: + @pytest.mark.asyncio + async def test_granted_valid_prompt_ok(self, validator): + res = await validator.validate_coordination_action( + action="escalate_to_user", + data={"prompt": "re-authenticate to x.com"}, + agent=_FakeAgent(can_escalate=True), + branch=None, + exec_state=None, + ) + assert res.is_valid + assert res.action_type == ActionType.ESCALATE_USER + assert res.parsed_response["prompt"] == "re-authenticate to x.com" + + @pytest.mark.asyncio + async def test_ungranted_rejected(self, validator): + res = await validator.validate_coordination_action( + action="escalate_to_user", + data={"prompt": "re-auth"}, + agent=_FakeAgent(can_escalate=False), + branch=None, + exec_state=None, + ) + assert not res.is_valid + assert res.error_category == ValidationErrorCategory.PERMISSION_ERROR.value + + @pytest.mark.asyncio + async def test_empty_prompt_rejected(self, validator): + res = await validator.validate_coordination_action( + action="escalate_to_user", + data={"prompt": ""}, + agent=_FakeAgent(can_escalate=True), + branch=None, + exec_state=None, + ) + assert not res.is_valid + assert res.error_category == ValidationErrorCategory.ACTION_ERROR.value + + @pytest.mark.asyncio + async def test_ask_user_gate_unaffected_by_grant(self, validator): + # ask_user stays gated on the topology edge, independent of can_escalate: + # no User edge → rejected even for a can_escalate-granted agent (AC-23/AC-24). + res = await validator.validate_coordination_action( + action="ask_user", + data={"question": "q"}, + agent=_FakeAgent(can_escalate=True), + branch=None, + exec_state=None, + ) + assert not res.is_valid + + +# ── Instruction surface (separate, grant-gated block) ─────────────────────── + +def _ctx( + *, + can_escalate_user: bool = False, + can_terminate_workflow: bool = False, + can_ask_user: bool = False, +) -> SystemPromptContext: + return SystemPromptContext( + agent=AgentContext( + name="A", goal="g", instruction="Do it.", tools={}, tools_schema=[] + ), + coordination=CoordinationContext( + can_terminate_workflow=can_terminate_workflow, + can_ask_user=can_ask_user, + can_escalate_user=can_escalate_user, + ), + ) + + +class TestEscalateInstructionSurface: + def _fmt(self): + from marsys.coordination.formats.json_format.format import JSONResponseFormat + + return JSONResponseFormat() + + def test_present_when_granted(self): + out = self._fmt()._build_escalate_instructions(_ctx(can_escalate_user=True)) + assert "escalate_to_user" in out + assert "ESCALATION" in out + + def test_absent_when_not_granted(self): + assert self._fmt()._build_escalate_instructions(_ctx(can_escalate_user=False)) == "" + + def test_present_even_with_no_completion_capability(self): + # AC-10/AC-22: a granted agent on a User-less, End-less topology — the + # topology-gated completion block is empty, but the escalate contract + # still appears (separate gate). + fmt = self._fmt() + ctx = _ctx( + can_escalate_user=True, + can_terminate_workflow=False, + can_ask_user=False, + ) + assert fmt._build_workflow_completion_instructions(ctx) == "" + assert "escalate_to_user" in fmt._build_escalate_instructions(ctx) + + def test_assembled_prompt_includes_escalate_when_granted(self): + # AC-20: the ASSEMBLED system prompt (not just the sub-builder) carries the + # escalate contract when granted. + prompt = self._fmt().build_complete_system_prompt(_ctx(can_escalate_user=True)) + assert "escalate_to_user" in prompt + + def test_assembled_prompt_omits_escalate_when_ungranted(self): + # AC-21: the assembled prompt omits the escalate contract when ungranted. + prompt = self._fmt().build_complete_system_prompt(_ctx(can_escalate_user=False)) + assert "escalate_to_user" not in prompt + + +# ── Translate seam: the REAL validate→_translate path (AC-16 / AC-17) ──────── +# The deterministic integration suite injects the ESCALATE_USER StepResult +# directly into DeterministicRuntime, bypassing _translate — so a mis-sourced +# prompt or wrong kind would not turn the suite red. These drive the real seam. + +class TestEscalateTranslate: + def _runtime(self): + from marsys.coordination.execution.real_runtime import RealRuntime + + g = Mock(spec=TopologyGraph) + g.get_next_agents = MagicMock(return_value=["End"]) + return RealRuntime( + registry=Mock(), step_executor=Mock(), + validator=ValidationProcessor(g), topology_graph=g, session_id="t", + ) + + def _marsys_result(self, prompt: str): + from types import SimpleNamespace + + return SimpleNamespace( + success=True, coordination_action="escalate_to_user", + coordination_data={"prompt": prompt}, tool_calls=None, response=None, + ) + + def _branch(self): + from marsys.coordination.execution.orchestrator_types import Branch + + return Branch(id="b1", current_agent="A", status="RUNNING", delivery_target="root") + + @pytest.mark.asyncio + async def test_translate_escalate_action_to_step(self): + # AC-16: the validated action translates to an ESCALATE_USER step. + # AC-17: its prompt is sourced from the validator output (not a stray local). + step = await self._runtime()._translate( + self._marsys_result("re-auth example.com"), + self._branch(), + _FakeAgent(can_escalate=True), + ) + assert step.kind == "ESCALATE_USER" + assert step.value == "re-auth example.com" + + @pytest.mark.asyncio + async def test_translate_ungranted_escalate_fails(self): + # An ungranted agent's escalate action is rejected by validation → the + # translate seam returns FAIL, never an ESCALATE_USER step (AC-3/AC-12). + step = await self._runtime()._translate( + self._marsys_result("re-auth"), + self._branch(), + _FakeAgent(can_escalate=False), + ) + assert step.kind == "FAIL" + + +# ── Negative space: the deferred vocabulary was NOT pre-built (AC-29) ───────── + +def test_actiontype_gained_exactly_escalate_user(): + # The action-type surface gains EXACTLY ESCALATE_USER — no pause/redirect/fail + # directive was smuggled in (anti-pattern #11 / AC-29). + assert set(ActionType.__members__) == { + "INVOKE_AGENT", "PARALLEL_INVOKE", "FINAL_RESPONSE", "TERMINATE_WORKFLOW", + "ASK_USER", "ESCALATE_USER", "END_CONVERSATION", "ERROR_RECOVERY", + "TERMINAL_ERROR", "AUTO_RETRY", + } + + +def test_stepkind_gained_exactly_escalate_user(): + # The step-kind surface gains EXACTLY ESCALATE_USER (AC-29 / AC-18). + from marsys.coordination.execution.orchestrator_types import StepKind + + assert set(StepKind.__args__) == { + "NOOP", "SINGLE_INVOKE", "PARALLEL_INVOKE", "FINAL_RESPONSE", + "ESCALATE_USER", "FAIL", + } + + +def test_default_agent_coordination_set_unchanged(): + # AC-27: a default (ungranted) agent's coordination tool set is EXACTLY the + # pre-FW18 set — escalate absent, the standard tools present unchanged. + names = { + s["function"]["name"] + for s in CoordinationToolSchemaBuilder.build_schemas( + next_agents=["Helper"], can_terminate_workflow=True, + can_ask_user=True, is_conversation_branch=True, + ) + } + assert names == {"invoke_agent", "terminate_workflow", "ask_user", "end_conversation"} + + +@pytest.mark.asyncio +async def test_ask_user_still_accepted_with_user_edge(): + # AC-24 (positive half): ask_user validation is unchanged — WITH a User edge it + # still validates, independent of can_escalate. + g = Mock(spec=TopologyGraph) + g.has_edge_to_usernode = MagicMock(return_value=True) + g.get_next_agents = MagicMock(return_value=["User"]) + res = await ValidationProcessor(g).validate_coordination_action( + action="ask_user", data={"question": "q"}, + agent=_FakeAgent(can_escalate=False), branch=None, exec_state=None, + ) + assert res.is_valid and res.action_type == ActionType.ASK_USER diff --git a/tests/integration/test_durable_hitl.py b/tests/integration/test_durable_hitl.py new file mode 100644 index 00000000..2316eb8e --- /dev/null +++ b/tests/integration/test_durable_hitl.py @@ -0,0 +1,257 @@ +"""Durable HITL — Orchestra public-surface round-trip (ADR-012). + +The cornerstone (AC-31): produce a REAL durable suspend (a UserNode flagged +durable in the topology *spec*), write it to a FileStorageBackend via the same +``_snapshot_and_write`` helper ``Orchestra.execute()`` uses on its awaiting-user +exit, then resume it in a FRESH ``Orchestra`` via ``resume_session(user_response=…)`` +— driving the resume agent through ``RealRuntime`` to a terminal ``OrchestraResult``. + +Mirrors ``test_pause_resume.py``'s real-agent resume pattern. The suspend half +uses ``DeterministicRuntime`` (A ``SINGLE_INVOKE``s the durable ``User`` node); the +resume half uses a deterministic stub agent (overrides ``_run``, no model call). +The execute()-driven awaiting-user exit and cost-across-resume are the gated live +test ``test_durable_hitl_live.py``. +""" +from __future__ import annotations + +import asyncio +import json as _json +import uuid as _uuid + +import pytest + +from marsys.coordination.config import ExecutionConfig +from marsys.coordination.execution.deterministic_runtime import DeterministicRuntime +from marsys.coordination.execution.orchestrator import Orchestrator +from marsys.coordination.execution.orchestrator_types import ( + ConvergencePolicy, + StepResult, + reset_ids, +) +from marsys.coordination.orchestra import Orchestra +from marsys.coordination.state import FileStorageBackend +from marsys.coordination.topology.core import Edge, Node, NodeKind, Topology + + +def _durable_spec() -> Topology: + """Start → A → User(durable) → B → End, durability declared in the spec via + the USER node's metadata (the workflow-definition path, founder option 2).""" + return Topology( + nodes=[ + Node(name="Start", kind=NodeKind.START), + Node(name="A", kind=NodeKind.AGENT), + Node(name="User", kind=NodeKind.USER, metadata={"durable": True}), + Node(name="B", kind=NodeKind.AGENT), + Node(name="End", kind=NodeKind.END), + ], + edges=[ + Edge(source="Start", target="A"), + Edge(source="A", target="User"), + Edge(source="User", target="B"), + Edge(source="B", target="End"), + ], + ) + + +def _stub_resume_agent_cls(): + """A deterministic resume agent 'B' that finalizes on its first tick — no + model call (same pattern test_pause_resume.py's real-agent test uses).""" + from marsys.agents import Agent + from marsys.agents.memory import Message, ToolCallMsg + from marsys.models import ModelConfig + + class _B(Agent): + def __init__(self): + super().__init__( + model_config=ModelConfig( + type="api", name="mock-model", provider="openai", api_key="mock-key", + ), + goal="finish", instruction="Finish immediately.", name="B", + ) + + async def _run(self, messages, request_context, run_mode="default", **kwargs): + cid = f"call_{_uuid.uuid4().hex[:8]}" + return Message( + role="assistant", content="done", name="B", + tool_calls=[ToolCallMsg( + id=cid, call_id=cid, type="function", + name="return_final_response", + arguments=_json.dumps({"response": "resumed-ok"}), + )], + ) + + return _B + + +def _ask_user_stub_cls(): + """A stub entry agent 'A' that emits an `ask_user` action on its first tick — + drives Orchestra.execute() to the durable User node through RealRuntime with no + model call (the validator maps the ask_user tool call → ASK_USER → + SINGLE_INVOKE('User'), gated on the A→User edge).""" + from marsys.agents import Agent + from marsys.agents.memory import Message, ToolCallMsg + from marsys.models import ModelConfig + + class _A(Agent): + def __init__(self): + super().__init__( + model_config=ModelConfig( + type="api", name="mock-model", provider="openai", api_key="mock-key", + ), + goal="ask", instruction="Ask the user to re-authenticate.", name="A", + ) + + async def _run(self, messages, request_context, run_mode="default", **kwargs): + cid = f"call_{_uuid.uuid4().hex[:8]}" + return Message( + role="assistant", content="asking", name="A", + tool_calls=[ToolCallMsg( + id=cid, call_id=cid, type="function", name="ask_user", + arguments=_json.dumps({"question": "Please re-authenticate"}), + )], + ) + + return _A + + +@pytest.mark.asyncio +async def test_execute_drives_durable_suspend_and_flags_metadata(tmp_path): + """AC-1/2/4/5/6/25: a REAL RealRuntime dispatch through Orchestra.execute() — + a stub agent emits ask_user on a spec-declared durable User node; execute() + takes the awaiting-user exit, writes the snapshot ITSELF before returning, and + flags both metadata observables; the run is not presented as finished.""" + from marsys.agents.registry import AgentRegistry + + reset_ids() + backend = FileStorageBackend(tmp_path / "snapshots") + sid = "exec-durable" + AgentRegistry.clear() + try: + a = _ask_user_stub_cls()() + AgentRegistry._test_agents = [a] + orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + out = await orch.execute( + task="go", topology=_durable_spec(), context={"session_id": sid}, + ) + # AC-1/AC-2: the public metadata observables (not just the internal sentinel). + assert out.metadata.get("paused") is True + assert out.metadata.get("awaiting_user") is True + # AC-6 intent: the run is suspended, not presented as a completed success. + assert out.success is False + assert out.final_response is None + # AC-5: execute() itself wrote the snapshot before returning (no external pause). + assert (tmp_path / "snapshots" / sid / "snapshot.json").is_file() + finally: + AgentRegistry.clear() + + +@pytest.mark.asyncio +async def test_pause_session_on_durable_parked_run_does_not_hang(tmp_path): + """AC-21: a run that durably suspended itself via execute() does NOT leave + pause_session hanging — the loop already exited at the durable boundary, so a + subsequent pause_session is an idempotent no-op (it finds the snapshot already + on disk), completing well within a timeout rather than blocking on a human.""" + from marsys.agents.registry import AgentRegistry + + reset_ids() + backend = FileStorageBackend(tmp_path / "snapshots") + sid = "exec-durable-pause" + AgentRegistry.clear() + try: + a = _ask_user_stub_cls()() + AgentRegistry._test_agents = [a] + orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + out = await orch.execute( + task="go", topology=_durable_spec(), context={"session_id": sid}, + ) + assert out.metadata.get("awaiting_user") is True + # The durable wait never blocked the loop, so pause_session can't hang on + # it — it idempotent-no-ops on the already-written snapshot. (Pre-durable, + # a UserNode-parked run blocked pause_session until the 300s timeout.) + await asyncio.wait_for(orch.pause_session(sid), timeout=5.0) + finally: + AgentRegistry.clear() + + +async def _suspend_durable_to_disk(backend, sid, spec) -> Orchestra: + """Drive a durable workload to its awaiting-user exit and persist the + snapshot the way execute() does. Returns the pause-side Orchestra.""" + pause_orch = Orchestra(agent_registry=None, storage_backend=backend) + pause_orch._build_topology_graph(spec, ExecutionConfig()) + # The shim-built durable UserNode has no handler bound, and needs none — the + # durable path skips the SYNC handler guard (it never invokes the handler). + rt = DeterministicRuntime() + rt.queue_agent("A", StepResult( + kind="SINGLE_INVOKE", next_agent="User", value="authenticate?", + )) + underlying = Orchestrator(pause_orch.topology_graph, rt, ConvergencePolicy()) + result = await underlying.run(task="go", entry_agent="A") + assert result.error == "awaiting_user", f"did not durably suspend: {result.error}" + await pause_orch._snapshot_and_write(sid, underlying) + return pause_orch + + +@pytest.mark.asyncio +async def test_orchestra_durable_round_trip(tmp_path): + """AC-5/12/14/25/31: a spec-declared durable user step suspends to disk via + the Orchestra surface and resumes in a FRESH Orchestra with the response, + driving the resume agent to a terminal success; the snapshot is discarded.""" + from marsys.agents.registry import AgentRegistry + + reset_ids() + backend = FileStorageBackend(tmp_path / "snapshots") + sid = "durable-rt" + snap_path = tmp_path / "snapshots" / sid / "snapshot.json" + + AgentRegistry.clear() + try: + agent = _stub_resume_agent_cls()() # auto-registers (weakref store) + AgentRegistry._test_agents = [agent] # keep a strong ref + + await _suspend_durable_to_disk(backend, sid, _durable_spec()) + assert snap_path.is_file() # AC-5 + + resume_orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + out = await resume_orch.resume_session( + sid, canonical_topology=_durable_spec(), user_response="AUTH_DONE", + ) + + assert out.success, f"durable resume did not complete: {out.error}" # AC-12/14 + assert out.final_response == "resumed-ok" # the real agent ran + assert out.metadata.get("resumed") is True + assert not snap_path.is_file() # AC-31 discarded + finally: + AgentRegistry.clear() + + +@pytest.mark.asyncio +async def test_multi_consumer_round_trip_no_spren(tmp_path): + """AC-30/31: the full durable round-trip from a plain consumer-style entry + point — construct Orchestra(FileStorageBackend), durably suspend, construct a + NEW Orchestra, resume_session(user_response=…), complete — with no Spren + coupling. (The grep-for-imports check is AC-30 proper; this proves the + consumer-style API path works standalone.)""" + from marsys.agents.registry import AgentRegistry + + reset_ids() + backend = FileStorageBackend(tmp_path / "snapshots") + sid = "multi-consumer" + + AgentRegistry.clear() + try: + agent = _stub_resume_agent_cls()() + AgentRegistry._test_agents = [agent] + + await _suspend_durable_to_disk(backend, sid, _durable_spec()) + + # A brand-new Orchestra (the "process B" consumer) resumes from disk. + consumer = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + listed = await consumer.list_paused_sessions() + assert any(m.session_id == sid for m in listed) + + out = await consumer.resume_session( + sid, canonical_topology=_durable_spec(), user_response="done", + ) + assert out.success, f"consumer-side resume failed: {out.error}" + finally: + AgentRegistry.clear() diff --git a/tests/integration/test_durable_hitl_live.py b/tests/integration/test_durable_hitl_live.py new file mode 100644 index 00000000..77d00f96 --- /dev/null +++ b/tests/integration/test_durable_hitl_live.py @@ -0,0 +1,167 @@ +"""LIVE (real-OAuth) durable HITL resume test (ADR-012) — the regression guard +for the FW17 bus-rebuild fix IN THE DURABLE PATH. + +Gated on an explicit opt-in (``SPREN_LIVE_LLM=1`` or ``MARSYS_LIVE_LLM=1``) plus a +configured anthropic-oauth profile — it spends real tokens, so it never runs in +CI (OAuth is local-only per ToS). Run it explicitly: + + SPREN_LIVE_LLM=1 uv run python -m pytest \ + packages/framework/tests/integration/test_durable_hitl_live.py -s + +Why this exists, and why it is NOT covered by the deterministic suite: the +deterministic tests resume a STUB agent (no model call). This test resumes a REAL +OAuth agent across a durable suspend — exercising OAuth resolution, a genuine +provider call, and the resumed dispatch running on the rebuilt EventBus (the FW17 +fix). A consumer re-attached via ``on_bus_rebuilt`` receives the resumed +real-agent dispatch's events, proving the bus rebuild carries the resumed run. + +(The USD cost path — a consumer subscribing ``LLMCallEvent`` — is exercised +end-to-end by Spren's tracing-enabled live test; ``LLMCallEvent`` is a *tracing* +event a bare Orchestra does not emit without a TraceCollector, so this test +asserts on the orchestrator's ``BranchCompletedEvent`` instead and prints the +``LLMCallEvent`` count for visibility.) + +The durable suspend itself is produced deterministically (reliable — no dependence +on an LLM choosing to invoke the User node); the LIVE half is the resume, which is +where the bus-rebuild fix lives. +""" +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + +from marsys.coordination.config import ExecutionConfig +from marsys.coordination.execution.deterministic_runtime import DeterministicRuntime +from marsys.coordination.execution.det_nodes import UserNode +from marsys.coordination.execution.orchestrator import Orchestrator +from marsys.coordination.execution.orchestrator_types import ( + ConvergencePolicy, + StepResult, + reset_ids, +) +from marsys.coordination.orchestra import Orchestra +from marsys.coordination.state import FileStorageBackend +from marsys.coordination.topology.core import Edge, Node, NodeKind, Topology + +_LIVE = os.environ.get("SPREN_LIVE_LLM") == "1" or os.environ.get("MARSYS_LIVE_LLM") == "1" +_OAUTH_CONFIGURED = (Path.home() / ".marsys" / "credentials.json").exists() + +pytestmark = pytest.mark.skipif( + not (_LIVE and _OAUTH_CONFIGURED), + reason="live OAuth test — set SPREN_LIVE_LLM=1 (or MARSYS_LIVE_LLM=1) with a " + "configured anthropic-oauth profile", +) + + +def _durable_spec() -> Topology: + return Topology( + nodes=[ + Node(name="Start", kind=NodeKind.START), + Node(name="A", kind=NodeKind.AGENT), + Node(name="User", kind=NodeKind.USER, metadata={"durable": True}), + Node(name="B", kind=NodeKind.AGENT), + Node(name="End", kind=NodeKind.END), + ], + edges=[ + Edge(source="Start", target="A"), + Edge(source="A", target="User"), + Edge(source="User", target="B"), + Edge(source="B", target="End"), + ], + ) + + +def _real_resume_agent(): + """A REAL resume agent 'B' on anthropic-oauth — it makes a genuine model call + on resume (no _run override), so the resumed dispatch emits a real + LLMCallEvent. claude-opus-4-8 with extended thinking requires temperature=1.0.""" + from marsys.agents import Agent + from marsys.models import ModelConfig + + return Agent( + model_config=ModelConfig( + type="api", name="claude-opus-4-8", provider="anthropic-oauth", + max_tokens=128, temperature=1.0, + ), + goal="acknowledge", + instruction="Reply with a one-sentence acknowledgement that " + "re-authentication is complete, then stop.", + name="B", + ) + + +@pytest.mark.asyncio +async def test_live_durable_resume_real_oauth_llm_event_across_rebuild(tmp_path): + """Resume a durable suspend with a REAL OAuth agent and assert a consumer + re-attached via on_bus_rebuilt receives the resumed dispatch's real + LLMCallEvent — the FW17 bus-rebuild fix, exercised in the durable path.""" + from marsys.agents.registry import AgentRegistry + + reset_ids() + backend = FileStorageBackend(tmp_path / "snapshots") + sid = "durable-live" + spec = _durable_spec() + + AgentRegistry.clear() + try: + agent = _real_resume_agent() # auto-registers + AgentRegistry._test_agents = [agent] # strong ref + + # ── Deterministic durable suspend → snapshot on disk. ── + pause_orch = Orchestra(agent_registry=None, storage_backend=backend) + pause_orch._build_topology_graph(spec, ExecutionConfig()) + for det in (pause_orch.topology_graph.det_nodes or {}).values(): + if isinstance(det, UserNode): + det.handler = object() + rt = DeterministicRuntime() + rt.queue_agent("A", StepResult( + kind="SINGLE_INVOKE", next_agent="User", value="authenticate?", + )) + underlying = Orchestrator(pause_orch.topology_graph, rt, ConvergencePolicy()) + susp = await underlying.run(task="go", entry_agent="A") + assert susp.error == "awaiting_user" + await pause_orch._snapshot_and_write(sid, underlying) + assert (tmp_path / "snapshots" / sid / "snapshot.json").is_file() + + # ── LIVE resume: a real OAuth call on the resumed dispatch must reach a + # consumer re-attached to the rebuilt bus via on_bus_rebuilt. ── + branch_events = [] + llm_events = [] + + def on_bus_rebuilt(bus): + # A consumer re-attaches to the rebuilt bus (the FW17 contract) and + # must then receive events from the resumed REAL-agent dispatch. + bus.subscribe("BranchCompletedEvent", lambda ev: branch_events.append(ev)) + bus.subscribe("LLMCallEvent", lambda ev: llm_events.append(ev)) + + resume_orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + out = await resume_orch.resume_session( + sid, + canonical_topology=_durable_spec(), + user_response="Re-authentication complete — please continue.", + on_bus_rebuilt=on_bus_rebuilt, + ) + + print(f"\n[live] durable resume: success={out.success} error={out.error!r} " + f"final={out.final_response!r} branch_events={len(branch_events)} " + f"llm_events={len(llm_events)}") + + # The durable run reconstructed from disk and drove a REAL OAuth agent to + # terminal — a genuine model response, not a stub. + assert out.error != "awaiting_user", "resume should not re-suspend here" + assert out.success, f"durable real-agent resume did not complete: {out.error}" + assert isinstance(out.final_response, str) and out.final_response.strip(), ( + f"resume agent produced no real model response: {out.final_response!r}" + ) + # A consumer re-attached via on_bus_rebuilt received the resumed dispatch's + # events — the FW17 bus rebuild carries the resumed REAL-agent run (the + # durable-path analogue of FW17's stub-agent resume test). + assert branch_events, ( + "a consumer re-attached via on_bus_rebuilt received no event from the " + "resumed real-agent dispatch — the bus rebuild did not carry the " + "resumed run (FW17 regression in the durable path)" + ) + finally: + AgentRegistry.clear() diff --git a/tests/integration/test_escalate_to_user.py b/tests/integration/test_escalate_to_user.py new file mode 100644 index 00000000..78860c56 --- /dev/null +++ b/tests/integration/test_escalate_to_user.py @@ -0,0 +1,217 @@ +"""Integration: the `escalate_to_user` control directive end to end (ADR-013). + +A *granted* agent on a topology with NO `User` node emits `escalate_to_user` → the +run durably suspends (paused-awaiting-user) → `resume_session(user_response)` +resumes the EMITTING agent to terminal. The durable suspend/resume is framework 16, +reused unchanged: escalate produces the SAME `pending_user_interaction` shape and +routes into it without a topology `User` node. + +Mirrors tests/integration/test_durable_hitl.py. The real emit→validate→translate +→route path is driven through `Orchestra.execute()` with a granted stub agent; the +resume half is driven from a scripted `ESCALATE_USER` step + a deterministic stub +agent (overrides `_run`, no model call). The full real round-trip with a live model +is the gated `test_escalate_to_user_live.py`. +""" +from __future__ import annotations + +import json as _json +import uuid as _uuid + +import pytest + +from marsys.coordination.config import ExecutionConfig +from marsys.coordination.execution.deterministic_runtime import DeterministicRuntime +from marsys.coordination.execution.orchestrator import Orchestrator +from marsys.coordination.execution.orchestrator_types import ( + ConvergencePolicy, + StepResult, + reset_ids, +) +from marsys.coordination.orchestra import Orchestra +from marsys.coordination.state import FileStorageBackend +from marsys.coordination.topology.core import Edge, Node, NodeKind, Topology + + +def _escalate_spec() -> Topology: + """Start → A → End — NO User node. A (granted can_escalate) escalates.""" + return Topology( + nodes=[ + Node(name="Start", kind=NodeKind.START), + Node(name="A", kind=NodeKind.AGENT), + Node(name="End", kind=NodeKind.END), + ], + edges=[ + Edge(source="Start", target="A"), + Edge(source="A", target="End"), + ], + ) + + +def _escalate_stub_cls(*, can_escalate: bool): + """Stub agent 'A' that emits escalate_to_user on its first tick (no model call). + Granted or not per `can_escalate` — the validator gate is what differs.""" + from marsys.agents import Agent + from marsys.agents.memory import Message, ToolCallMsg + from marsys.models import ModelConfig + + class _A(Agent): + def __init__(self): + super().__init__( + model_config=ModelConfig( + type="api", name="mock-model", provider="openai", api_key="mock-key", + ), + goal="escalate", instruction="Escalate to the user.", name="A", + can_escalate=can_escalate, + ) + + async def _run(self, messages, request_context, run_mode="default", **kwargs): + cid = f"call_{_uuid.uuid4().hex[:8]}" + return Message( + role="assistant", content="escalating", name="A", + tool_calls=[ToolCallMsg( + id=cid, call_id=cid, type="function", name="escalate_to_user", + arguments=_json.dumps({"prompt": "Please re-authenticate to example.com"}), + )], + ) + + return _A + + +def _finalize_stub_cls(): + """Resume agent 'A' (the emitter) that finalizes on its first tick. It echoes + whether the user_response reached it as input (AC-7): returns 'resumed-ok' only + if it saw the re-auth marker, else 'resumed-NO-INPUT'.""" + from marsys.agents import Agent + from marsys.agents.memory import Message, ToolCallMsg + from marsys.models import ModelConfig + + class _A(Agent): + def __init__(self): + super().__init__( + model_config=ModelConfig( + type="api", name="mock-model", provider="openai", api_key="mock-key", + ), + goal="finish", instruction="Finish immediately.", name="A", + can_escalate=True, + ) + + async def _run(self, messages, request_context, run_mode="default", **kwargs): + blob = repr(messages) + repr(request_context) + saw_response = "REAUTH_DONE" in blob + cid = f"call_{_uuid.uuid4().hex[:8]}" + return Message( + role="assistant", content="done", name="A", + tool_calls=[ToolCallMsg( + id=cid, call_id=cid, type="function", name="return_final_response", + arguments=_json.dumps( + {"response": "resumed-ok" if saw_response else "resumed-NO-INPUT"} + ), + )], + ) + + return _A + + +@pytest.mark.asyncio +async def test_execute_drives_escalate_suspend(tmp_path): + """AC-1..AC-5: a REAL RealRuntime dispatch — a granted stub agent emits + escalate_to_user on a User-LESS topology; execute() takes the awaiting-user + exit, writes the snapshot itself, and flags both metadata observables; the run + is not presented as finished. This is the case the has_edge_to_usernode gate + forbids for ask_user.""" + from marsys.agents.registry import AgentRegistry + + reset_ids() + backend = FileStorageBackend(tmp_path / "snapshots") + sid = "exec-escalate" + AgentRegistry.clear() + try: + a = _escalate_stub_cls(can_escalate=True)() + AgentRegistry._test_agents = [a] + orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + out = await orch.execute( + task="go", topology=_escalate_spec(), context={"session_id": sid}, + ) + assert out.metadata.get("paused") is True + assert out.metadata.get("awaiting_user") is True + assert out.success is False + assert out.final_response is None + assert (tmp_path / "snapshots" / sid / "snapshot.json").is_file() + finally: + AgentRegistry.clear() + + +@pytest.mark.asyncio +async def test_ungranted_escalate_does_not_suspend(tmp_path): + """AC-3: an UNGRANTED agent emitting escalate_to_user is rejected by validation + — the run does NOT durably suspend (no awaiting-user) and is not presented as a + success.""" + from marsys.agents.registry import AgentRegistry + + reset_ids() + backend = FileStorageBackend(tmp_path / "snapshots") + sid = "exec-escalate-ungranted" + AgentRegistry.clear() + try: + a = _escalate_stub_cls(can_escalate=False)() + AgentRegistry._test_agents = [a] + orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + out = await orch.execute( + task="go", topology=_escalate_spec(), context={"session_id": sid}, + ) + assert out.metadata.get("awaiting_user") is not True + assert out.success is False + finally: + AgentRegistry.clear() + + +async def _suspend_escalate_to_disk(backend, sid, spec) -> Orchestra: + """Drive an escalation to its awaiting-user exit via the ESCALATE_USER step and + persist the snapshot the way execute() does. Returns the pause-side Orchestra. + resume_agent is set by the orchestrator arm to the emitting agent (A).""" + pause_orch = Orchestra(agent_registry=None, storage_backend=backend) + pause_orch._build_topology_graph(spec, ExecutionConfig()) + rt = DeterministicRuntime() + rt.queue_agent( + "A", StepResult(kind="ESCALATE_USER", value="Please re-authenticate") + ) + underlying = Orchestrator(pause_orch.topology_graph, rt, ConvergencePolicy()) + result = await underlying.run(task="go", entry_agent="A") + assert result.error == "awaiting_user", f"did not durably suspend: {result.error}" + await pause_orch._snapshot_and_write(sid, underlying) + return pause_orch + + +@pytest.mark.asyncio +async def test_escalate_durable_round_trip(tmp_path): + """AC-2/AC-6..AC-9: an ESCALATE_USER suspend persists to disk and resumes in a + FRESH Orchestra via resume_session(user_response=…), driving the EMITTING agent + (A) — not a successor — to terminal success with the response delivered as its + input; the snapshot is discarded.""" + from marsys.agents.registry import AgentRegistry + + reset_ids() + backend = FileStorageBackend(tmp_path / "snapshots") + sid = "escalate-rt" + snap_path = tmp_path / "snapshots" / sid / "snapshot.json" + + AgentRegistry.clear() + try: + agent = _finalize_stub_cls()() # the emitter 'A', re-run on resume + AgentRegistry._test_agents = [agent] + + await _suspend_escalate_to_disk(backend, sid, _escalate_spec()) + assert snap_path.is_file() # AC-4 + + resume_orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + out = await resume_orch.resume_session( + sid, canonical_topology=_escalate_spec(), user_response="REAUTH_DONE", + ) + + assert out.success, f"escalate resume did not complete: {out.error}" # AC-2/AC-8 + # 'resumed-ok' only if the user_response reached the resumed agent (AC-7). + assert out.final_response == "resumed-ok" + assert out.metadata.get("resumed") is True + assert not snap_path.is_file() # AC-9 (discarded) + finally: + AgentRegistry.clear() diff --git a/tests/integration/test_escalate_to_user_live.py b/tests/integration/test_escalate_to_user_live.py new file mode 100644 index 00000000..ad691047 --- /dev/null +++ b/tests/integration/test_escalate_to_user_live.py @@ -0,0 +1,192 @@ +"""LIVE (real-OAuth) escalate_to_user test (ADR-013). + +Gated on an explicit opt-in (``SPREN_LIVE_LLM=1`` or ``MARSYS_LIVE_LLM=1``) plus a +configured OAuth profile — it spends real tokens, so it never runs in CI (OAuth is +local-only per ToS). Run it explicitly: + + SPREN_LIVE_LLM=1 uv run python -m pytest \ + packages/framework/tests/integration/test_escalate_to_user_live.py -s + +Why this exists, beyond the deterministic suite (which uses STUB agents, no model +call): + +1. ``test_live_escalate_real_emit_and_resume`` — the genuine end-to-end re-auth + shape with a REAL model: a granted agent, given the escalate_to_user tool + its + instruction block, *chooses* to call escalate_to_user (proving the schema + + instruction surface + validation work against a live model), the run durably + suspends, and on resume the SAME real agent — told re-auth is complete — drives + to a real terminal response. The deterministic tests can't prove a live model + will actually emit the directive. +2. ``test_live_escalate_resume_bus_rebuild`` — a RELIABLE suspend (scripted + ESCALATE_USER, no dependence on the model choosing to escalate) + a LIVE resume, + asserting a consumer re-attached via ``on_bus_rebuilt`` receives the resumed + real-agent dispatch's events (the FW17 bus-rebuild contract, in the escalate + path). +""" +from __future__ import annotations + +import os +from pathlib import Path + +import pytest + +from marsys.coordination.config import ExecutionConfig +from marsys.coordination.execution.deterministic_runtime import DeterministicRuntime +from marsys.coordination.execution.orchestrator import Orchestrator +from marsys.coordination.execution.orchestrator_types import ( + ConvergencePolicy, + StepResult, + reset_ids, +) +from marsys.coordination.orchestra import Orchestra +from marsys.coordination.state import FileStorageBackend +from marsys.coordination.topology.core import Edge, Node, NodeKind, Topology + +_LIVE = os.environ.get("SPREN_LIVE_LLM") == "1" or os.environ.get("MARSYS_LIVE_LLM") == "1" +_OAUTH_CONFIGURED = (Path.home() / ".marsys" / "credentials.json").exists() + +pytestmark = pytest.mark.skipif( + not (_LIVE and _OAUTH_CONFIGURED), + reason="live OAuth test — set SPREN_LIVE_LLM=1 (or MARSYS_LIVE_LLM=1) with a " + "configured OAuth profile (~/.marsys/credentials.json)", +) + + +def _escalate_spec() -> Topology: + """Start → A → End — NO User node. A (granted) escalates, then on resume + terminates.""" + return Topology( + nodes=[ + Node(name="Start", kind=NodeKind.START), + Node(name="A", kind=NodeKind.AGENT), + Node(name="End", kind=NodeKind.END), + ], + edges=[ + Edge(source="Start", target="A"), + Edge(source="A", target="End"), + ], + ) + + +def _real_reauth_agent(): + """A REAL granted agent on anthropic-oauth that escalates while unauthenticated + and finalizes once told re-authentication is complete — the genuine S62 shape. + claude-opus-4-8 with extended thinking requires temperature=1.0.""" + from marsys.agents import Agent + from marsys.models import ModelConfig + + return Agent( + model_config=ModelConfig( + type="api", name="claude-opus-4-8", provider="anthropic-oauth", + max_tokens=256, temperature=1.0, + ), + goal="Operate a tool that requires the user to be authenticated.", + instruction=( + "You operate a tool that requires the user to be authenticated. " + "If you have NOT been told that authentication is complete, you cannot " + "proceed — call the `escalate_to_user` tool with a prompt asking the " + "user to re-authenticate. Once you are told re-authentication is " + "complete, reply with a one-sentence confirmation and call " + "`return_final_response` to finish. Do not do anything else." + ), + name="A", + can_escalate=True, + ) + + +@pytest.mark.asyncio +async def test_live_escalate_real_emit_and_resume(tmp_path): + """End-to-end with a REAL model: the agent CHOOSES to call escalate_to_user + (schema + instruction + validation against a live model) → durable suspend → + resume the same real agent (told re-auth complete) → real terminal response.""" + from marsys.agents.registry import AgentRegistry + + reset_ids() + backend = FileStorageBackend(tmp_path / "snapshots") + sid = "escalate-live-e2e" + AgentRegistry.clear() + try: + agent = _real_reauth_agent() + AgentRegistry._test_agents = [agent] + + # ── LIVE suspend: a real model emits escalate_to_user. ── + orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + susp = await orch.execute( + task="Begin the task.", topology=_escalate_spec(), + context={"session_id": sid}, + ) + print(f"\n[live] escalate suspend: paused={susp.metadata.get('paused')} " + f"awaiting_user={susp.metadata.get('awaiting_user')} success={susp.success}") + assert susp.metadata.get("awaiting_user") is True, ( + "the live model did not emit escalate_to_user → no durable suspend " + f"(success={susp.success}, error={susp.error!r})" + ) + assert (tmp_path / "snapshots" / sid / "snapshot.json").is_file() + + # ── LIVE resume: the same real agent, told re-auth is complete, finishes. ── + resume_orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + out = await resume_orch.resume_session( + sid, canonical_topology=_escalate_spec(), + user_response="Re-authentication is complete — please continue and finish.", + ) + print(f"[live] escalate resume: success={out.success} error={out.error!r} " + f"final={out.final_response!r}") + assert out.error != "awaiting_user", "resume should not re-suspend here" + assert out.success, f"escalate real-agent resume did not complete: {out.error}" + assert isinstance(out.final_response, str) and out.final_response.strip(), ( + f"resumed agent produced no real model response: {out.final_response!r}" + ) + finally: + AgentRegistry.clear() + + +@pytest.mark.asyncio +async def test_live_escalate_resume_bus_rebuild(tmp_path): + """A RELIABLE scripted ESCALATE_USER suspend + a LIVE resume: a consumer + re-attached via on_bus_rebuilt receives the resumed real-agent dispatch's + events (the FW17 bus-rebuild contract, in the escalate path).""" + from marsys.agents.registry import AgentRegistry + + reset_ids() + backend = FileStorageBackend(tmp_path / "snapshots") + sid = "escalate-live-resume" + spec = _escalate_spec() + AgentRegistry.clear() + try: + agent = _real_reauth_agent() # resume re-runs the emitter 'A' + AgentRegistry._test_agents = [agent] + + # Deterministic durable suspend via the ESCALATE_USER step → snapshot. + pause_orch = Orchestra(agent_registry=None, storage_backend=backend) + pause_orch._build_topology_graph(spec, ExecutionConfig()) + rt = DeterministicRuntime() + rt.queue_agent( + "A", StepResult(kind="ESCALATE_USER", value="Please re-authenticate."), + ) + underlying = Orchestrator(pause_orch.topology_graph, rt, ConvergencePolicy()) + susp = await underlying.run(task="go", entry_agent="A") + assert susp.error == "awaiting_user" + await pause_orch._snapshot_and_write(sid, underlying) + assert (tmp_path / "snapshots" / sid / "snapshot.json").is_file() + + branch_events = [] + + def on_bus_rebuilt(bus): + bus.subscribe("BranchCompletedEvent", lambda ev: branch_events.append(ev)) + + resume_orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + out = await resume_orch.resume_session( + sid, canonical_topology=_escalate_spec(), + user_response="Re-authentication is complete — please continue and finish.", + on_bus_rebuilt=on_bus_rebuilt, + ) + print(f"\n[live] escalate resume(bus): success={out.success} " + f"final={out.final_response!r} branch_events={len(branch_events)}") + assert out.success, f"escalate live resume did not complete: {out.error}" + assert isinstance(out.final_response, str) and out.final_response.strip() + assert branch_events, ( + "a consumer re-attached via on_bus_rebuilt received no event from the " + "resumed real-agent dispatch (FW17 regression in the escalate path)" + ) + finally: + AgentRegistry.clear() diff --git a/tests/integration/test_pause_resume.py b/tests/integration/test_pause_resume.py index 11fafb01..a86b254b 100644 --- a/tests/integration/test_pause_resume.py +++ b/tests/integration/test_pause_resume.py @@ -1022,3 +1022,300 @@ async def test_orchestra_resume_session_round_trip_through_state_snapshot(tmp_pa assert "snapshot.max_steps" in src, ( "resume_session must construct Orchestrator with snapshot.max_steps" ) + + +# ─── resume ergonomics — canonical_topology + on_bus_rebuilt ───────────────── +# A cross-process consumer (e.g. Spren S61) reconstructs a paused run from disk +# in a fresh process: it supplies the canonical topology (bound internally via +# the shared _build_topology_graph) and re-attaches its custom EventBus +# subscribers via on_bus_rebuilt. Both params are additive, optional, +# keyword-only, and default to today's behavior. + +from marsys.coordination.config import ExecutionConfig as _ExecutionConfig +from marsys.agents.exceptions import StateError as _StateError + +_FW17_TOPO = {"agents": ["A", "B"], "flows": ["A -> B"]} + + +def _fw17_canonical(tmp_path, name): + """A fresh canonical Topology built from a fixed dict — the form + resume_session(canonical_topology=) expects (what execute() coerces).""" + src = _make_orchestra(tmp_path / name) + return src._ensure_topology(dict(_FW17_TOPO)) + + +def _fw17_reference_digest(tmp_path): + """The (digest, topology_graph) execute() would produce for _FW17_TOPO via + the shared _build_topology_graph path — the AC-2 reference.""" + src = _make_orchestra(tmp_path / "fw17-ref") + src._build_topology_graph(src._ensure_topology(dict(_FW17_TOPO)), _ExecutionConfig()) + return src._compute_topology_digest(), src.topology_graph + + +async def _fw17_write_snapshot(orch, sid, *, topology_digest): + """Write an empty-state snapshot with a chosen topology_digest.""" + now = datetime.now(tz=timezone.utc) + snapshot = StateSnapshot( + framework_version=marsys.__version__, + session_id=sid, + topology_digest=topology_digest, + created_at=now, + paused_at=now, + branches={}, + barriers={}, + convergence_barriers={}, + runnable=[], + fire_queue=[], + completed_emitted=[], + user_interactions=[], + user_interaction_inflight=False, + ) + await orch.storage_backend.write( + f"{sid}/snapshot.json", snapshot.model_dump_json().encode("utf-8") + ) + + +@pytest.mark.asyncio +async def test_fw17_canonical_topology_binds_equivalent_graph(tmp_path): + """AC-1 + AC-2: resume_session(canonical_topology=) binds the topology + internally — a fresh Orchestra with nothing pre-set gets PAST the + RESUME_NO_TOPOLOGY guard (it raises the *digest* error instead, proving the + bind ran before the guard), and the bound graph equals execute()'s. We + deliberately mismatch the snapshot digest so resume raises right after the + bind, before dispatch — the bind, not the dispatch, is what FW17 adds.""" + ref_digest, ref_graph = _fw17_reference_digest(tmp_path) + + orch = _make_orchestra(tmp_path / "run") + sid = "fw17-bind" + await _fw17_write_snapshot(orch, sid, topology_digest="deliberately-wrong") + + assert getattr(orch, "topology_graph", None) is None # nothing pre-set + with pytest.raises(IncompatibleSnapshotError): + await orch.resume_session(sid, canonical_topology=_fw17_canonical(tmp_path, "c1")) + + # AC-1: it passed the RESUME_NO_TOPOLOGY guard (raised the digest error). + # AC-2: the internal bind produced the graph execute() would build — equal + # digest, equal nodes AND edges, and the explicit `A -> B` flow actually + # survived analyze+shim (not just == another call of the same helper). + assert orch._compute_topology_digest() == ref_digest + assert set(orch.topology_graph.nodes) == set(ref_graph.nodes) + bound_edges = {(e.source, e.target) for e in orch.topology_graph.edges} + assert bound_edges == {(e.source, e.target) for e in ref_graph.edges} + assert ("A", "B") in bound_edges + assert {"A", "B"} <= set(orch.topology_graph.nodes) + + +@pytest.mark.asyncio +async def test_fw17_no_topology_raises_and_callback_not_fired(tmp_path): + """AC-6: with neither a pre-bound topology nor canonical_topology, resume + raises RESUME_NO_TOPOLOGY and on_bus_rebuilt does NOT fire.""" + orch = _make_orchestra(tmp_path / "run") + sid = "fw17-no-topo" + await _fw17_write_snapshot(orch, sid, topology_digest="x") + + fired = [] + with pytest.raises(_StateError) as exc: + await orch.resume_session(sid, on_bus_rebuilt=lambda bus: fired.append(bus)) + assert exc.value.error_code == "RESUME_NO_TOPOLOGY" + assert fired == [] + + +@pytest.mark.asyncio +async def test_fw17_mismatched_topology_raises_and_callback_not_fired(tmp_path): + """AC-7 + AC-8: a canonical_topology whose digest differs from the + snapshot's raises IncompatibleSnapshotError after the bind, and + on_bus_rebuilt does NOT fire (the digest check aborts first).""" + orch = _make_orchestra(tmp_path / "run") + sid = "fw17-mismatch" + await _fw17_write_snapshot(orch, sid, topology_digest="not-the-real-digest") + + fired = [] + with pytest.raises(IncompatibleSnapshotError): + await orch.resume_session( + sid, + canonical_topology=_fw17_canonical(tmp_path, "c2"), + on_bus_rebuilt=lambda bus: fired.append(bus), + ) + assert fired == [] + + +@pytest.mark.asyncio +async def test_fw17_on_bus_rebuilt_fires_once_after_preconditions(tmp_path): + """AC-3 + AC-4: on a resume whose preconditions pass, on_bus_rebuilt fires + exactly once with the rebuilt bus, and a subscriber attached inside it is + live on the bus the resumed run uses.""" + ref_digest, _ = _fw17_reference_digest(tmp_path) + + orch = _make_orchestra(tmp_path / "run") + sid = "fw17-cb" + await _fw17_write_snapshot(orch, sid, topology_digest=ref_digest) + + seen = [] + received = [] + + def cb(bus): + seen.append(bus) + bus.subscribe("_FW17ProbeEvent", lambda ev: received.append(ev)) + + # on_bus_rebuilt fires post-precondition, BEFORE restore/dispatch. The + # crafted empty-state snapshot then can't drive resume() to completion + # (no restored branches) — irrelevant to this test, which asserts only that + # the callback fired once with the live run bus. + try: + await orch.resume_session( + sid, canonical_topology=_fw17_canonical(tmp_path, "c3"), on_bus_rebuilt=cb + ) + except RuntimeError: + pass + + assert len(seen) == 1 # fired exactly once + assert seen[0] is orch.event_bus # with the rebuilt bus + + class _FW17ProbeEvent: + pass + + await orch.event_bus.emit(_FW17ProbeEvent()) + assert len(received) == 1 # subscriber is live on the run's bus + + +@pytest.mark.asyncio +async def test_fw17_on_bus_rebuilt_raising_propagates(tmp_path): + """AC-9: a raising on_bus_rebuilt callback propagates out of resume_session + (fails loudly, not swallowed).""" + ref_digest, _ = _fw17_reference_digest(tmp_path) + + orch = _make_orchestra(tmp_path / "run") + sid = "fw17-cb-raises" + await _fw17_write_snapshot(orch, sid, topology_digest=ref_digest) + + class _Boom(RuntimeError): + pass + + def cb(bus): + raise _Boom("subscriber wiring failed") + + with pytest.raises(_Boom): + await orch.resume_session( + sid, canonical_topology=_fw17_canonical(tmp_path, "c4"), on_bus_rebuilt=cb + ) + + +@pytest.mark.asyncio +async def test_fw17_new_params_are_keyword_only_and_optional(tmp_path): + """AC-10: both new params are keyword-only (the `*` gates both — a positional + second arg is a TypeError) and optional (omitting both is the unchanged + pre-FW17 path).""" + orch = _make_orchestra(tmp_path / "run") + sid = "fw17-kw" + await _fw17_write_snapshot(orch, sid, topology_digest="x") + + # keyword-only: a positional second arg is rejected — the `*` gates BOTH + # canonical_topology and on_bus_rebuilt. + with pytest.raises(TypeError): + await orch.resume_session(sid, _fw17_canonical(tmp_path, "c5")) # positional + + # optional / back-compat: omitting both args is the unchanged pre-FW17 path — + # with no topology bound it raises RESUME_NO_TOPOLOGY exactly as before. + with pytest.raises(_StateError) as exc: + await orch.resume_session(sid) + assert exc.value.error_code == "RESUME_NO_TOPOLOGY" + + +@pytest.mark.asyncio +async def test_fw17_canonical_topology_resume_drives_real_agent_to_terminal(tmp_path): + """AC-1 (terminal) + AC-4 (real post-dispatch event), end-to-end: a fresh + Orchestra resumes a real on-disk snapshot via resume_session(canonical_topology=), + binds the topology internally, dispatches a REAL agent through RealRuntime to a + terminal OrchestraResult, and a subscriber attached inside on_bus_rebuilt + receives a run event emitted by the resumed dispatch (not a test-injected emit). + + The agent is a deterministic stub (overrides _run, no LLM) — the same pattern + test_real_runtime_smoke uses to exercise RealRuntime without a model call. + """ + import json as _json + import uuid as _uuid + + from marsys.agents import Agent + from marsys.agents.memory import Message, ToolCallMsg + from marsys.agents.registry import AgentRegistry + from marsys.models import ModelConfig + + class _FinalAgent(Agent): + """Returns a final response on its first (only) tick — no model call.""" + + def __init__(self): + super().__init__( + model_config=ModelConfig( + type="api", name="mock-model", provider="openai", api_key="mock-key", + ), + goal="finish", instruction="Finish immediately.", name="A", + ) + + async def _run(self, messages, request_context, run_mode="default", **kwargs): + cid = f"call_{_uuid.uuid4().hex[:8]}" + return Message( + role="assistant", content="done", name="A", + tool_calls=[ToolCallMsg( + id=cid, call_id=cid, type="function", + name="return_final_response", + arguments=_json.dumps({"response": "done"}), + )], + ) + + topo_dict = {"agents": ["User", "A"], "flows": ["User -> A", "A -> User"]} + backend = FileStorageBackend(tmp_path / "snapshots") + sid = "fw17-complete" + + AgentRegistry.clear() + try: + agent = _FinalAgent() # auto-registers (weakref store); keep a strong ref + AgentRegistry._test_agents = [agent] + + # ── Create a REAL paused snapshot: a branch runnable at A. The gated + # runtime NOOPs A's tick (it never calls the agent), so the snapshot has + # a runnable branch at A — exactly what a mid-flight pause produces. ── + pause_orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + canonical = pause_orch._ensure_topology(dict(topo_dict)) + pause_orch.canonical_topology = canonical + pause_orch.topology_graph = pause_orch.topology_analyzer.analyze(canonical) + runtime = _GatedAsyncRuntime() + underlying = Orchestrator(pause_orch.topology_graph, runtime, ConvergencePolicy()) + pause_orch._active_orchestrators[sid] = underlying + run_task = asyncio.create_task(underlying.run(task="go", entry_agent="A")) + while runtime.tick_count < 1 and not run_task.done(): + await asyncio.sleep(0.01) + pause_task = asyncio.create_task(pause_orch.pause_session(sid)) + await asyncio.sleep(0) + runtime.release() + await pause_task + await run_task + + # ── Resume in a FRESH Orchestra via the new canonical_topology param. + # resume_session builds RealRuntime, which dispatches the REAL agent A. ── + resume_orch = Orchestra(agent_registry=AgentRegistry, storage_backend=backend) + seen, received = [], [] + + def cb(bus): + seen.append(bus) + # BranchCompletedEvent is emitted by the orchestrator to the rebuilt + # bus during the resumed dispatch — proof that a subscriber + # re-attached via on_bus_rebuilt receives real post-callback run events. + bus.subscribe("BranchCompletedEvent", lambda ev: received.append(ev)) + + result = await resume_orch.resume_session( + sid, + canonical_topology=resume_orch._ensure_topology(dict(topo_dict)), + on_bus_rebuilt=cb, + ) + + _bus_events = [type(e).__name__ for e in resume_orch.event_bus.events] + assert result.success, f"resume did not reach a terminal success: {result.error}" + assert result.final_response == "done" # AC-1: the real agent actually ran + assert len(seen) == 1 # AC-3: callback fired exactly once + assert seen[0] is resume_orch.event_bus # AC-3: with the rebuilt run bus + assert received, ( # AC-4: a REAL dispatch event reached the + "a subscriber attached inside on_bus_rebuilt received no run event from " + f"the resumed dispatch; bus_events={_bus_events}; final={result.final_response!r}" + ) + finally: + AgentRegistry.clear() diff --git a/tests/models/test_adapter_harmonize.py b/tests/models/test_adapter_harmonize.py index 6f4fc9c5..f7432017 100644 --- a/tests/models/test_adapter_harmonize.py +++ b/tests/models/test_adapter_harmonize.py @@ -87,8 +87,16 @@ def test_anthropic_format_request_payload_drops_message_level_name(): assert set(m.keys()) <= {"role", "content"}, ( f"Anthropic message must carry only role/content, got {sorted(m.keys())}" ) - # content is preserved on the rebuilt messages - assert payload["messages"][-1]["content"] == "Here are the verified facts ..." + # content is preserved on the rebuilt messages. Read as TEXT, not as an exact + # container: the tail message carries the prompt-cache breakpoint, which promotes + # a plain string to a one-element text block (same bytes to the model, and the + # only shape a marker can ride). What this test is about is the dropped `name`. + tail_content = payload["messages"][-1]["content"] + tail_text = ( + tail_content if isinstance(tail_content, str) + else "".join(b.get("text", "") for b in tail_content) + ) + assert tail_text == "Here are the verified facts ..." assert payload["messages"][-1]["role"] == "assistant" @@ -287,10 +295,16 @@ def test_anthropic_truncation_empty_harmonizes_valid_with_placeholder(): # empty `model_context_window_exceeded`, or a stream that closed without a terminal — # used to construct HarmonizedResponse(content=None), die in the model validator, and # surface as an UNKNOWN ValidationError with the provider's terminal signal destroyed -# (the boot-replay crash). Contract now: deterministic truncation (max_tokens AND -# model_context_window_exceeded) takes the placeholder; every OTHER empty terminal -# raises a typed ModelAPIError classified by stop_reason. stop_details is nullable -# decoration: captured by the readers, surfaced in messages, never keyed on. +# (the boot-replay crash). +# +# Contract now, in three arms: +# - deterministic truncation (max_tokens AND model_context_window_exceeded) takes +# the placeholder; +# - `end_turn` is a SILENT TURN — the model ran to natural completion and chose to +# say nothing. A success, harmonized to content="" (see below); +# - every OTHER empty terminal raises a typed ModelAPIError classified by stop_reason. +# stop_details is nullable decoration: captured by the readers, surfaced in messages, +# never keyed on. def _empty_oauth_raw(stop_reason, stop_details=None, **overrides): @@ -334,16 +348,34 @@ def test_oauth_empty_refusal_without_stop_details_still_classifies(): assert "category" not in str(err) -def test_oauth_empty_end_turn_raises_typed_with_recovery_action(): - """Empty end_turn is non-retryable (Anthropic: don't retry empty responses - without modification); the suggested action carries the documented recovery.""" - with pytest.raises(ModelAPIError) as exc: - _oauth_adapter().harmonize_response(_empty_oauth_raw("end_turn"), request_start_time=0.0) - err = exc.value - assert err.classification == APIErrorClassification.EMPTY_COMPLETION.value - assert err.is_retryable is False - assert "end_turn" in str(err) - assert "modif" in (err.suggested_action or "").lower() +def test_oauth_empty_end_turn_is_a_silent_turn_not_an_error(): + """A SILENT TURN, not a failure. An agent instructed to stay quiet when it has + nothing to report ends the turn with zero content blocks and stop_reason + 'end_turn'; the provider bills that as a success. It must harmonize to the + empty-STRING content shape (the validator rejects None, never ""), so a caller + that supports a contentless reply gets one instead of a raised turn. + + This INVERTS the original 2026-06-12 assertion (empty end_turn → non-retryable + ModelAPIError). That contract was wrong: it classified a success as a fault and + terminally killed every silent turn, which is a behaviour the prompt layer + explicitly asks for.""" + resp = _oauth_adapter().harmonize_response( + _empty_oauth_raw("end_turn"), request_start_time=0.0 + ) + assert resp.content == "" # the empty-string shape, NOT None + assert resp.tool_calls == [] + assert resp.metadata.stop_reason == "end_turn" + assert resp.metadata.finish_reason == "end_turn" + + +def test_anthropic_empty_end_turn_is_a_silent_turn_not_an_error(): + """The API-key twin holds the same contract — one provider, one behaviour.""" + raw = {"role": "assistant", "content": [], "stop_reason": "end_turn", + "usage": {"input_tokens": 1, "output_tokens": 2}} + resp = _adapter().harmonize_response(raw, request_start_time=0.0) + assert resp.content == "" + assert resp.tool_calls == [] + assert resp.metadata.stop_reason == "end_turn" @pytest.mark.parametrize("stop_reason", [None, "stop_sequence", "never_seen_terminal"]) diff --git a/tests/models/test_async_error_body.py b/tests/models/test_async_error_body.py index 299adb6b..045a95b1 100644 --- a/tests/models/test_async_error_body.py +++ b/tests/models/test_async_error_body.py @@ -97,12 +97,19 @@ def test_body_less_400_classifies_invalid_request(): assert result.classification["is_retryable"] is False -def test_true_connection_error_no_status_stays_unknown(): - """A genuine connection failure (no HTTP response, no status anywhere): - classification stays unknown and the message is the exception text. - This is the real no-regression case for 'no response at all'.""" +def test_true_connection_error_no_status_classifies_retryable_network(): + """A genuine connection failure (no HTTP response, no status anywhere) is a + TRANSIENT, retryable network error — it must classify as ``network_error`` so + the caller's retry policy heals it, not the ``unknown`` non-retryable default. + + This INVERTS the original assertion (connection error -> 'unknown'). That + default terminally dropped turns on any transport blip (a DNS hiccup: + '[Errno 11001] getaddrinfo failed'); a cannot-connect is definitionally + transient. The exception text is still preserved verbatim in the message — + only the classification is corrected.""" result = _adapter().handle_api_error( ConnectionError("Cannot connect to host api.anthropic.com"), response=None ) assert "Cannot connect to host" in result.error - assert result.classification["category"] == "unknown" + assert result.classification["category"] == "network_error" + assert result.classification["is_retryable"] is True diff --git a/tests/models/test_claude5_payload_shape.py b/tests/models/test_claude5_payload_shape.py new file mode 100644 index 00000000..1d25cc0a --- /dev/null +++ b/tests/models/test_claude5_payload_shape.py @@ -0,0 +1,242 @@ +"""Payload shaping for reasoning-capable Claude models (no network). + +Claude Opus 5 / Sonnet 5 (and Opus 4.7/4.8 before them) progressively removed +the sampling parameters and the fixed thinking budget from the Messages API. +Both are hard 400s rather than ignored fields, so a wrong payload does not +degrade — the turn fails outright. These tests pin the shape per model so a +regression surfaces here instead of as a live 400 on a user's first turn. + +Measured against the live API when written: + * ``temperature`` → 400 on opus-4-7/4-8/opus-5/sonnet-5 + * ``thinking.type="enabled"`` → 400 on Bedrock for those models + * ``thinking.type="adaptive"`` → accepted + * ``output_config.format`` → 400 on every Bedrock path +""" + +import json + +import pytest + +from marsys.models.adapters.anthropic import ( + AnthropicAdapter, + _anthropic_model_rejects_temperature, + _anthropic_model_requires_adaptive_thinking, +) +from marsys.models.adapters.bedrock import ( + BedrockAdapter, + bedrock_base_url, + normalize_bedrock_model_id, +) + +MESSAGES = [{"role": "user", "content": "hi"}] +SCHEMA = { + "type": "object", + "properties": {"a": {"type": "string"}}, + "required": ["a"], +} + + +def _api(model_name: str) -> AnthropicAdapter: + return AnthropicAdapter( + model_name=model_name, + api_key="not-a-real-key", + base_url="https://api.anthropic.com/v1", + max_tokens=8192, + ) + + +# --- capability predicates --------------------------------------------------- + + +@pytest.mark.parametrize( + "model_name, adaptive", + [ + ("claude-opus-5", True), + ("claude-sonnet-5", True), + ("claude-opus-4-8", True), + ("claude-opus-4-7", True), + ("claude-fable-5", True), + # Spelling must not change the answer: capability is the model's, not the + # id format's. + ("anthropic.claude-opus-5", True), + ("us.anthropic.claude-opus-5", True), + ("anthropic/claude-sonnet-5", True), + ("CLAUDE-OPUS-5", True), + ("claude-opus-5-20260401", True), # dated snapshots inherit + # Legacy models keep the fixed-budget shape. + ("claude-sonnet-4-6", False), + ("claude-opus-4-6", False), + ("claude-haiku-4-5-20251001", False), + ("", False), + ], +) +def test_adaptive_thinking_predicate(model_name, adaptive): + assert _anthropic_model_requires_adaptive_thinking(model_name) is adaptive + # The two deprecations landed together on every model in this family. + assert _anthropic_model_rejects_temperature(model_name) is adaptive + + +# --- thinking shape --------------------------------------------------------- + + +@pytest.mark.parametrize("model_name", ["claude-opus-5", "claude-sonnet-5"]) +def test_claude5_gets_adaptive_thinking_not_a_budget(model_name): + payload = _api(model_name).format_request_payload(MESSAGES, thinking_budget=8192) + assert payload["thinking"] == {"type": "adaptive"} + assert "budget_tokens" not in json.dumps(payload) + + +def test_legacy_model_keeps_fixed_budget(): + payload = _api("claude-sonnet-4-6").format_request_payload( + MESSAGES, thinking_budget=4096 + ) + assert payload["thinking"]["type"] == "enabled" + assert payload["thinking"]["budget_tokens"] == 4096 + + +@pytest.mark.parametrize("model_name", ["claude-opus-5", "claude-sonnet-4-6"]) +def test_no_thinking_key_when_budget_is_zero(model_name): + payload = _api(model_name).format_request_payload(MESSAGES, thinking_budget=0) + assert "thinking" not in payload + + +# --- temperature ------------------------------------------------------------ + + +@pytest.mark.parametrize("model_name", ["claude-opus-5", "claude-sonnet-5"]) +def test_claude5_never_gets_temperature(model_name): + """Rejected with thinking on OR off — the model refuses the key outright.""" + for budget in (8192, 0): + payload = _api(model_name).format_request_payload( + MESSAGES, thinking_budget=budget, temperature=0.7 + ) + assert "temperature" not in payload + + +# --- effort ----------------------------------------------------------------- + + +def test_effort_rides_output_config_for_adaptive_models(): + payload = _api("claude-opus-5").format_request_payload( + MESSAGES, thinking_budget=8192, reasoning_effort="HIGH" + ) + assert payload["output_config"]["effort"] == "high" + + +def test_effort_not_sent_to_legacy_models(): + """Older models 400 on the key, so it must not leak to them.""" + payload = _api("claude-sonnet-4-6").format_request_payload( + MESSAGES, thinking_budget=4096, reasoning_effort="high" + ) + assert "output_config" not in payload + + +def test_effort_not_sent_when_thinking_is_off(): + """Opus 5 rejects effort above 'high' with thinking disabled; sending none + avoids the interaction entirely.""" + payload = _api("claude-opus-5").format_request_payload( + MESSAGES, thinking_budget=0, reasoning_effort="xhigh" + ) + assert "output_config" not in payload + + +@pytest.mark.parametrize("effort", ["low", "medium", "high", "xhigh", "max"]) +def test_config_layer_accepts_every_effort_tier_the_models_support(effort): + """`xhigh`/`max` are real tiers on these models; if ModelConfig rejects them + the effort plumbing is unreachable for exactly the settings that matter most.""" + from marsys.models.models import ModelConfig + + config = ModelConfig( + type="api", + name="claude-opus-5", + provider="anthropic", + api_key="k", + reasoning_effort=effort, + ) + assert config.reasoning_effort == effort + + +def test_effort_and_schema_share_one_output_config(): + """The API takes exactly one output_config — a naive assign drops effort.""" + payload = _api("claude-opus-5").format_request_payload( + MESSAGES, thinking_budget=8192, reasoning_effort="low", response_schema=SCHEMA + ) + assert payload["output_config"]["effort"] == "low" + assert payload["output_config"]["format"]["type"] == "json_schema" + + +# --- Bedrock ---------------------------------------------------------------- + + +@pytest.mark.parametrize( + "given, expected", + [ + ("claude-opus-5", "anthropic.claude-opus-5"), + ("anthropic.claude-opus-5", "anthropic.claude-opus-5"), # idempotent + ("us.anthropic.claude-opus-5", "anthropic.claude-opus-5"), + ("eu.anthropic.claude-sonnet-5", "anthropic.claude-sonnet-5"), + ("anthropic/claude-sonnet-5", "anthropic.claude-sonnet-5"), + ("", ""), + ], +) +def test_bedrock_model_id_normalization(given, expected): + assert normalize_bedrock_model_id(given) == expected + + +def test_bedrock_base_url_carries_region(): + assert bedrock_base_url("eu-west-1") == ( + "https://bedrock-mantle.eu-west-1.api.aws/anthropic/v1" + ) + + +def test_bedrock_uses_bearer_auth_not_api_key_header(): + headers = BedrockAdapter(model_name="claude-opus-5", api_key="tok").get_headers() + assert headers["Authorization"] == "Bearer tok" + assert "x-api-key" not in headers + + +def test_bedrock_endpoint_is_the_messages_path(): + adapter = BedrockAdapter(model_name="claude-opus-5", api_key="tok") + assert adapter.get_endpoint_url().endswith("/anthropic/v1/messages") + + +def test_bedrock_degrades_schema_into_the_prompt(): + """`output_config.format` is rejected on Bedrock, so a schema request must + reach the model as prompt text rather than as an illegal field.""" + adapter = BedrockAdapter(model_name="claude-opus-5", api_key="tok") + payload = adapter.format_request_payload(MESSAGES, response_schema=SCHEMA) + assert "format" not in payload.get("output_config", {}) + assert "JSON Schema" in str(payload["messages"][-1]["content"]) + # The real schema travels, not just a "please emit JSON" nudge. + assert '"properties"' in str(payload["messages"][-1]["content"]) + + +def test_bedrock_reports_the_prefixed_id_not_the_bare_echo(): + """`metadata.model` is what cost meters price on. Bedrock echoes a bare id + (`claude-sonnet-5`) for a request made with `anthropic.claude-sonnet-5`, and a + rate table keyed by Bedrock ids finds no rate for the bare form — pricing the + whole provider at zero, silently. Reporting the requested id keeps the meter + honest.""" + adapter = BedrockAdapter(model_name="claude-sonnet-5", api_key="tok") + assert adapter.report_model_id("claude-sonnet-5") == "anthropic.claude-sonnet-5" + # Haiku resolves to a dated snapshot in the first-party namespace — also not + # a Bedrock id, so it must not be reported either. + haiku = BedrockAdapter(model_name="claude-haiku-4-5", api_key="tok") + assert haiku.report_model_id("claude-haiku-4-5-20251001") == "anthropic.claude-haiku-4-5" + assert haiku.report_model_id(None) == "anthropic.claude-haiku-4-5" + + +def test_first_party_adapter_reports_the_echoed_id(): + """First-party echoes stay in the caller's namespace, so the echo is + preferred — it resolves an alias to the concrete snapshot served.""" + adapter = _api("claude-sonnet-4-6") + assert adapter.report_model_id("claude-sonnet-4-6-20260101") == "claude-sonnet-4-6-20260101" + assert adapter.report_model_id(None) == "claude-sonnet-4-6" + + +def test_first_party_adapter_still_uses_native_structured_output(): + payload = _api("claude-opus-5").format_request_payload( + MESSAGES, response_schema=SCHEMA + ) + assert payload["output_config"]["format"]["type"] == "json_schema" + assert payload["output_config"]["format"]["schema"]["additionalProperties"] is False diff --git a/tests/models/test_deferred_tool_loading.py b/tests/models/test_deferred_tool_loading.py new file mode 100644 index 00000000..40231806 --- /dev/null +++ b/tests/models/test_deferred_tool_loading.py @@ -0,0 +1,187 @@ +"""Deferred tool loading (session 17) — per-adapter request-payload shapes (no network). + +A per-tool ``defer_loading: true`` rides the OpenAI-shaped tool dict top-level. Each adapter +maps it onto its provider-native tool + auto-adds that provider's tool-search built-in so deferred +tools are discovered on demand (their schemas stay out of the billed/cached prefix). The two +providers without the feature handle it explicitly: openrouter STRIPS the flag (it would otherwise +reach the wire), google WARNS (it already drops the flag). The load-bearing guarantees: + +- **Additive / identity:** with nothing deferred, each adapter's tool payload is byte-identical to + before (no ``defer_loading`` key, no search tool). +- **Cache preservation:** the ``tools=`` prefix is byte-identical across a discovery round-trip — + the discovered tool rides the message tail, never the ``tools=`` prefix (so the prompt cache holds). +""" +import pytest + +from marsys.models.adapters.anthropic import AnthropicAdapter +from marsys.models.adapters.anthropic_oauth import AnthropicOAuthAdapter +from marsys.models.adapters.google import GoogleAdapter +from marsys.models.adapters.openai import OpenAIAdapter +from marsys.models.adapters.openai_oauth import OpenAIOAuthAdapter +from marsys.models.adapters.openrouter import OpenRouterAdapter + +MSGS = [{"role": "user", "content": "hi"}] + + +def _tool(name="get_weather", *, defer=False): + t = { + "type": "function", + "function": { + "name": name, + "description": f"{name} description", + "parameters": {"type": "object", "properties": {}}, + }, + } + if defer: + t["defer_loading"] = True + return t + + +# --- adapter fixtures (OAuth ones get patched creds so __init__ touches no keychain/network) --- + + +@pytest.fixture +def anthropic(): + return AnthropicAdapter(model_name="claude-sonnet-4-6", api_key="x", base_url="https://api.anthropic.com/v1") + + +@pytest.fixture +def anthropic_oauth(monkeypatch): + monkeypatch.setattr(AnthropicOAuthAdapter, "_load_claude_credentials", + lambda self, path=None: {"access_token": "fake"}) + return AnthropicOAuthAdapter(model_name="claude-sonnet-4-6", auto_refresh=False) + + +@pytest.fixture +def openai(): + return OpenAIAdapter(model_name="gpt-5.4", api_key="x", base_url="https://api.openai.com/v1") + + +@pytest.fixture +def openai_oauth(monkeypatch): + monkeypatch.setattr(OpenAIOAuthAdapter, "_load_codex_credentials", + lambda self, path=None: {"access_token": "fake", "account_id": "acct"}) + return OpenAIOAuthAdapter(model_name="gpt-5.4", auto_refresh=False) + + +@pytest.fixture +def openrouter(): + return OpenRouterAdapter(model_name="anthropic/claude-sonnet-4-6", api_key="x", base_url="https://openrouter.ai/api/v1") + + +@pytest.fixture +def google(): + return GoogleAdapter(model_name="gemini-3.5-flash", api_key="x", base_url="https://generativelanguage.googleapis.com") + + +def _names(tools): + return [t.get("name") for t in tools if t.get("type") != "tool_search" and not str(t.get("type", "")).startswith("tool_search_tool")] + + +def _by_name(tools, name): + return next((t for t in tools if t.get("name") == name), None) + + +# --- Anthropic (api-key + OAuth): defer_loading maps + the regex search tool is added --- + + +def test_anthropic_apikey_maps_defer_loading_and_adds_search_tool(anthropic): + tools = anthropic.format_request_payload( + MSGS, tools=[_tool("get_weather", defer=True), _tool("core_tool")] + )["tools"] + assert _by_name(tools, "get_weather")["defer_loading"] is True + assert "defer_loading" not in _by_name(tools, "core_tool") # non-deferred tool unchanged + assert any(t.get("type") == "tool_search_tool_regex_20251119" for t in tools) # search tool added + + +def test_anthropic_apikey_nothing_deferred_is_identical(anthropic): + tools = anthropic.format_request_payload(MSGS, tools=[_tool("a"), _tool("b")])["tools"] + assert _names(tools) == ["a", "b"] + assert all("defer_loading" not in t for t in tools) + assert not any(str(t.get("type", "")).startswith("tool_search_tool") for t in tools) # no search tool + + +def test_anthropic_oauth_maps_defer_loading_and_adds_search_tool(anthropic_oauth): + tools = anthropic_oauth.format_request_payload( + MSGS, tools=[_tool("get_weather", defer=True), _tool("core_tool")] + )["tools"] + assert _by_name(tools, "get_weather")["defer_loading"] is True + assert "defer_loading" not in _by_name(tools, "core_tool") + assert any(t.get("type") == "tool_search_tool_regex_20251119" for t in tools) + + +def test_anthropic_oauth_nothing_deferred_is_identical(anthropic_oauth): + tools = anthropic_oauth.format_request_payload(MSGS, tools=[_tool("a"), _tool("b")])["tools"] + assert not any(str(t.get("type", "")).startswith("tool_search_tool") for t in tools) + assert all("defer_loading" not in t for t in tools) + + +# --- OpenAI Responses (api-key + OAuth): defer_loading maps + the tool_search built-in is added --- + + +def test_openai_apikey_maps_defer_loading_and_adds_tool_search(openai): + tools = openai.format_request_payload( + MSGS, tools=[_tool("get_weather", defer=True), _tool("core_tool")] + )["tools"] + assert _by_name(tools, "get_weather")["defer_loading"] is True + assert "defer_loading" not in _by_name(tools, "core_tool") + assert any(t.get("type") == "tool_search" for t in tools) + + +def test_openai_apikey_nothing_deferred_is_identical(openai): + tools = openai.format_request_payload(MSGS, tools=[_tool("a"), _tool("b")])["tools"] + assert not any(t.get("type") == "tool_search" for t in tools) + assert all("defer_loading" not in t for t in tools) + + +def test_openai_oauth_maps_defer_loading_and_adds_tool_search(openai_oauth): + tools = openai_oauth.format_request_payload( + MSGS, tools=[_tool("get_weather", defer=True), _tool("core_tool")] + )["tools"] + assert _by_name(tools, "get_weather")["defer_loading"] is True + assert any(t.get("type") == "tool_search" for t in tools) + + +def test_openai_oauth_nothing_deferred_is_identical(openai_oauth): + tools = openai_oauth.format_request_payload(MSGS, tools=[_tool("a"), _tool("b")])["tools"] + assert not any(t.get("type") == "tool_search" for t in tools) + + +# --- providers without the feature: openrouter STRIPS, google WARNS (no silent behavior change) --- + + +def test_openrouter_strips_defer_loading_and_warns(openrouter): + with pytest.warns(Warning, match="defer_loading"): + tools = openrouter.format_request_payload(MSGS, tools=[_tool("get_weather", defer=True)])["tools"] + # the flag must NOT reach the wire (verbatim forward would 400 on some providers) + assert all("defer_loading" not in t for t in tools) + + +def test_openrouter_nothing_deferred_forwards_unchanged(openrouter): + src = [_tool("a"), _tool("b")] + tools = openrouter.format_request_payload(MSGS, tools=src)["tools"] + assert tools == src # byte-identical verbatim forward + + +def test_google_warns_on_deferred(google): + with pytest.warns(Warning, match="defer_loading"): + google.format_request_payload(MSGS, tools=[_tool("get_weather", defer=True)]) + + +# --- cache preservation: the tools= prefix is byte-stable across a discovery round-trip --- + + +def test_anthropic_cache_prefix_stable_across_discovery_roundtrip(anthropic): + tools = [_tool("get_weather", defer=True), _tool("core_tool")] + turn1 = anthropic.format_request_payload([{"role": "user", "content": "weather in Paris?"}], tools=tools) + # The model searched + called the discovered tool; that discovery rides the MESSAGES, not tools=. + turn2 = anthropic.format_request_payload( + [ + {"role": "user", "content": "weather in Paris?"}, + {"role": "assistant", "content": "searching", + "tool_calls": [{"id": "c1", "function": {"name": "get_weather", "arguments": "{}"}}]}, + {"role": "tool", "tool_call_id": "c1", "name": "get_weather", "content": "{}"}, + ], + tools=tools, + ) + assert turn1["tools"] == turn2["tools"] # tools= prefix byte-identical → prompt cache holds diff --git a/tests/models/test_oauth_claude5_payload_shape.py b/tests/models/test_oauth_claude5_payload_shape.py new file mode 100644 index 00000000..45d8bfbf --- /dev/null +++ b/tests/models/test_oauth_claude5_payload_shape.py @@ -0,0 +1,75 @@ +"""Payload shaping on the Anthropic OAuth leg for Claude 5 (no network). + +The OAuth adapter builds its own payload, so the capability shaping proven for +the API-key adapter does not carry over for free. This leg previously set +``temperature`` unconditionally, which is a live 400 on every reasoning-capable +model (Opus 4.7/4.8 included, not just the Claude 5 pair). + +Note the two legs are NOT identical by measurement: a fixed ``budget_tokens`` is +still accepted here for Claude 5 while Bedrock rejects it. Adaptive is sent +anyway — it is accepted on both and is the shape the models are tuned for — but +that is a deliberate choice, not a forced one. +""" + +import pytest + +from marsys.models.adapters.anthropic_oauth import AnthropicOAuthAdapter + +MESSAGES = [{"role": "user", "content": "hi"}] + + +def _oauth(model_name: str, *, budget: int = 0, enable: bool = False): + """Build the adapter without touching the credentials file on disk.""" + adapter = AnthropicOAuthAdapter.__new__(AnthropicOAuthAdapter) + adapter.model_name = AnthropicOAuthAdapter.MODEL_ALIASES.get(model_name, model_name) + adapter.max_tokens = 8192 + adapter.temperature = 0.7 + adapter.enable_thinking = enable + adapter.thinking_budget = budget + return adapter + + +@pytest.mark.parametrize("model_name", ["claude-opus-5", "claude-sonnet-5"]) +def test_claude5_gets_adaptive_thinking(model_name): + payload = _oauth(model_name).format_request_payload(MESSAGES, thinking_budget=8192) + assert payload["thinking"] == {"type": "adaptive"} + + +@pytest.mark.parametrize( + "model_name", ["claude-opus-5", "claude-sonnet-5", "claude-opus-4-8", "claude-opus-4-7"] +) +def test_reasoning_models_never_get_temperature(model_name): + """The regression that mattered: this leg used to always send temperature, + so 4.7/4.8 were already 400ing before Claude 5 existed.""" + for budget in (8192, 0): + payload = _oauth(model_name).format_request_payload( + MESSAGES, thinking_budget=budget, temperature=0.7 + ) + assert "temperature" not in payload, (model_name, budget) + + +def test_legacy_model_keeps_budget_and_temperature(): + legacy = _oauth("claude-sonnet-4-6") + thinking = legacy.format_request_payload(MESSAGES, thinking_budget=4096) + assert thinking["thinking"] == {"type": "enabled", "budget_tokens": 4096} + plain = legacy.format_request_payload(MESSAGES, thinking_budget=0, temperature=0.3) + assert plain["temperature"] == 0.3 + assert "thinking" not in plain + + +def test_effort_rides_output_config_for_claude5(): + payload = _oauth("claude-opus-5").format_request_payload( + MESSAGES, thinking_budget=8192, reasoning_effort="XHigh" + ) + assert payload["output_config"]["effort"] == "xhigh" + + +def test_short_aliases_resolve_and_shape_as_claude5(): + """`opus`/`sonnet` now point at the Claude 5 generation, so alias callers + must get the Claude 5 payload shape too.""" + for alias in ("opus", "sonnet"): + payload = _oauth(alias).format_request_payload( + MESSAGES, thinking_budget=8192, temperature=0.7 + ) + assert payload["thinking"] == {"type": "adaptive"}, alias + assert "temperature" not in payload, alias diff --git a/tests/models/test_oauth_supported_models.py b/tests/models/test_oauth_supported_models.py index 625abccd..0d136f7a 100644 --- a/tests/models/test_oauth_supported_models.py +++ b/tests/models/test_oauth_supported_models.py @@ -11,12 +11,19 @@ def test_anthropic_oauth_supports_current_opus_generations(): - for model in ("claude-opus-4-8", "claude-opus-4-7", "claude-sonnet-4-6"): + for model in ( + "claude-opus-5", + "claude-sonnet-5", + "claude-opus-4-8", + "claude-opus-4-7", + "claude-sonnet-4-6", + ): assert model in AnthropicOAuthAdapter.SUPPORTED_MODELS, model -def test_anthropic_oauth_opus_alias_resolves_to_4_8(): - assert AnthropicOAuthAdapter.MODEL_ALIASES["opus"] == "claude-opus-4-8" +def test_anthropic_oauth_short_aliases_resolve_to_current_generation(): + assert AnthropicOAuthAdapter.MODEL_ALIASES["opus"] == "claude-opus-5" + assert AnthropicOAuthAdapter.MODEL_ALIASES["sonnet"] == "claude-sonnet-5" def test_openai_oauth_supports_current_gpt_generations(): diff --git a/tests/models/test_prompt_cache_breakpoint.py b/tests/models/test_prompt_cache_breakpoint.py new file mode 100644 index 00000000..e7366d91 --- /dev/null +++ b/tests/models/test_prompt_cache_breakpoint.py @@ -0,0 +1,504 @@ +"""Prompt-cache observability + the unconditional conversation-tail breakpoint. + +No network. Two things are pinned here, both measured against the live API before +being written down (a write pass reporting ``cache_creation_input_tokens`` and a +read pass reporting ``cache_read_input_tokens`` on both the OAuth and Bedrock +endpoints): + +* **Usage is observable.** ``input_tokens`` is the *uncached remainder*, so a + caller sizing a conversation or pricing a call needs the two cache figures + alongside it. Both Anthropic legs now harmonize them; every other provider + reports nothing and must harmonize to None without raising. +* **Every request writes a cacheable prefix.** The breakpoint rides the last + content block of the last message on every call — the platform's multi-turn + pattern — because a cache READ only exists where an earlier request WROTE an + entry. Adapter-owned and unconditional: the payload builder is the only layer + that knows the rendered block layout, and a forgettable caller opt-in is how + this ends up paying full price forever. +""" + +import json + +import pytest + +from marsys.models.adapters.anthropic import ( + AnthropicAdapter, + mark_conversation_tail_for_cache, +) +from marsys.models.adapters.anthropic_oauth import AnthropicOAuthAdapter +from marsys.models.adapters.bedrock import BedrockAdapter +from marsys.models.adapters.google import GoogleAdapter +from marsys.models.adapters.openai import OpenAIAdapter +from marsys.models.adapters.streaming import AnthropicStreamAccumulator +from marsys.models.response_models import UsageInfo + +MESSAGES = [{"role": "user", "content": "hi"}] +EPHEMERAL = {"type": "ephemeral"} + + +def _api(model_name: str = "claude-opus-5") -> AnthropicAdapter: + return AnthropicAdapter( + model_name=model_name, + api_key="not-a-real-key", + base_url="https://api.anthropic.com/v1", + max_tokens=8192, + ) + + +def _oauth(model_name: str = "claude-sonnet-4-6", **kwargs) -> AnthropicOAuthAdapter: + """An OAuth adapter without touching the credentials file on disk.""" + adapter = AnthropicOAuthAdapter.__new__(AnthropicOAuthAdapter) + adapter.model_name = AnthropicOAuthAdapter.MODEL_ALIASES.get(model_name, model_name) + adapter.max_tokens = kwargs.get("max_tokens", 8192) + adapter.temperature = kwargs.get("temperature", 0.7) + adapter.enable_thinking = kwargs.get("enable_thinking", False) + adapter.thinking_budget = kwargs.get("thinking_budget", 0) + adapter.auto_refresh = False + adapter.access_token = "not-a-real-token" + adapter.credentials = {"access_token": "not-a-real-token"} + adapter._credentials_path = "unused" + return adapter + + +def _markers(payload) -> int: + """Every ``cache_control`` marker anywhere in the payload.""" + def walk(node): + if isinstance(node, dict): + return (1 if "cache_control" in node else 0) + sum( + walk(v) for k, v in node.items() if k != "cache_control" + ) + if isinstance(node, list): + return sum(walk(v) for v in node) + return 0 + + return walk(payload) + + +def _marked_blocks(payload) -> list: + out = [] + for msg in payload.get("messages", []): + content = msg.get("content") + if isinstance(content, list): + out.extend(b for b in content if isinstance(b, dict) and "cache_control" in b) + system = payload.get("system") + if isinstance(system, list): + out.extend(b for b in system if isinstance(b, dict) and "cache_control" in b) + return out + + +# === AC-1 / AC-7 — the harmonized usage shape =============================== + + +def test_usage_exposes_both_cache_fields_defaulting_to_none(): + """AC-1: optional ints, default None.""" + usage = UsageInfo(prompt_tokens=10, completion_tokens=2) + assert usage.cache_read_input_tokens is None + assert usage.cache_creation_input_tokens is None + + +def test_a_response_reporting_no_cache_keeps_its_total_tokens(): + """AC-1: adding the fields must not move ``total_tokens`` for a response that + reports no cache activity — the pre-session value is prompt+completion.""" + usage = UsageInfo(prompt_tokens=100, completion_tokens=40) + assert usage.total_tokens == 140 + # And the cache-aware reading degenerates to the plain prompt count. + assert usage.full_prompt_tokens == 100 + + +def test_full_prompt_tokens_is_the_sum_of_all_three(): + """The whole point of the two new fields: ``prompt_tokens`` is only the + uncached remainder, so the real prompt is the sum. Measured live: a ~1227-token + prompt reported input_tokens=8 with cache_creation_input_tokens=1219.""" + usage = UsageInfo( + prompt_tokens=8, completion_tokens=4, + cache_creation_input_tokens=1219, cache_read_input_tokens=0, + ) + assert usage.full_prompt_tokens == 1227 + read = UsageInfo( + prompt_tokens=8, completion_tokens=4, + cache_creation_input_tokens=0, cache_read_input_tokens=1219, + ) + assert read.full_prompt_tokens == 1227 + + +def test_apikey_leg_populates_both_cache_fields_from_raw_usage(): + """AC-2. Figures are the live write-pass shape.""" + resp = { + "content": [{"type": "text", "text": "OK"}], + "stop_reason": "end_turn", + "model": "claude-opus-5", + "usage": { + "input_tokens": 12, "output_tokens": 4, + "cache_creation_input_tokens": 1564, "cache_read_input_tokens": 0, + }, + } + usage = _api().harmonize_response(resp, 0.0).metadata.usage + assert usage.prompt_tokens == 12 + assert usage.cache_creation_input_tokens == 1564 + assert usage.cache_read_input_tokens == 0 + assert usage.full_prompt_tokens == 1576 + + +def test_oauth_leg_populates_both_cache_fields_from_raw_usage(): + """AC-3. Figures are the live read-pass shape.""" + raw = { + "text": "OK", "thinking": "", "tool_use": [], + "stop_reason": "end_turn", "model": "claude-sonnet-4-6", "id": "msg_x", + "usage": { + "input_tokens": 8, "output_tokens": 4, + "cache_creation_input_tokens": 0, "cache_read_input_tokens": 1219, + }, + } + usage = _oauth().harmonize_response(raw, 0.0).metadata.usage + assert usage.prompt_tokens == 8 + assert usage.cache_read_input_tokens == 1219 + assert usage.cache_creation_input_tokens == 0 + assert usage.full_prompt_tokens == 1227 + + +def test_bedrock_inherits_the_cache_figures(): + """The production leg on this install is Bedrock, which subclasses the api-key + adapter — so the cache figures must arrive there without a second mapping.""" + resp = { + "content": [{"type": "text", "text": "OK"}], + "stop_reason": "end_turn", + "model": "claude-opus-5", + "usage": { + "input_tokens": 12, "output_tokens": 4, + "cache_creation_input_tokens": 0, "cache_read_input_tokens": 1564, + }, + } + adapter = BedrockAdapter(model_name="claude-opus-5", api_key="tok") + usage = adapter.harmonize_response(resp, 0.0).metadata.usage + assert usage.cache_read_input_tokens == 1564 + assert usage.full_prompt_tokens == 1576 + + +@pytest.mark.parametrize( + "adapter, raw", + [ + ( + OpenAIAdapter(model_name="gpt-4o", api_key="k", base_url="https://x/v1"), + { + # Responses-API shape (what this adapter speaks). + "output": [{ + "type": "message", "role": "assistant", "status": "completed", + "content": [{"type": "output_text", "text": "OK"}], + }], + "model": "gpt-4o", + "usage": {"input_tokens": 10, "output_tokens": 2, "total_tokens": 12}, + }, + ), + ( + GoogleAdapter(model_name="gemini-2.0-flash", api_key="k", + base_url="https://x/v1beta"), + { + "candidates": [{"content": {"parts": [{"text": "OK"}]}, + "finishReason": "STOP"}], + "usageMetadata": {"promptTokenCount": 10, "candidatesTokenCount": 2, + "totalTokenCount": 12}, + }, + ), + ], + ids=["openai", "google"], +) +def test_a_provider_reporting_no_cache_harmonizes_to_none_without_raising(adapter, raw): + """AC-7: every non-Anthropic adapter — both fields None, no exception.""" + usage = adapter.harmonize_response(raw, 0.0).metadata.usage + assert usage is not None + assert usage.cache_read_input_tokens is None + assert usage.cache_creation_input_tokens is None + assert usage.full_prompt_tokens == (usage.prompt_tokens or 0) + + +# === AC-4 / AC-5 — usage survives a SPLIT stream ============================ + +# The grammar, as observed on the wire this session: `message_start` carries the +# input-side figures (and the only copy of the cache-TTL breakdown), `message_delta` +# the final output count. Both must fold into one dict. +_START = { + "type": "message_start", + "message": { + "id": "msg_01", "model": "claude-opus-5", "role": "assistant", + "usage": { + "input_tokens": 12, "cache_creation_input_tokens": 1564, + "cache_read_input_tokens": 0, + "cache_creation": {"ephemeral_5m_input_tokens": 1564, + "ephemeral_1h_input_tokens": 0}, + "output_tokens": 1, + }, + }, +} +_BODY = [ + {"type": "content_block_start", "index": 0, + "content_block": {"type": "text", "text": ""}}, + {"type": "content_block_delta", "index": 0, + "delta": {"type": "text_delta", "text": "OK"}}, + {"type": "content_block_stop", "index": 0}, +] +_DELTA_OUTPUT_ONLY = { + "type": "message_delta", "delta": {"stop_reason": "end_turn"}, + "usage": {"output_tokens": 4}, +} + + +def test_apikey_accumulator_preserves_cache_figures_across_a_split_stream(): + """AC-4: the message-start figures survive the message-delta event.""" + acc = AnthropicStreamAccumulator() + for event in [_START, *_BODY, _DELTA_OUTPUT_ONLY]: + acc.feed(event) + usage = _api().harmonize_response(acc.to_rest_response(), 0.0).metadata.usage + assert usage.prompt_tokens == 12 + assert usage.completion_tokens == 4 + assert usage.cache_creation_input_tokens == 1564 + assert usage.cache_read_input_tokens == 0 + + +def _drive_oauth_reader(events: list[dict]) -> dict: + """Feed the OAuth adapter's hand-rolled reader without any transport. + + The reader is inline in ``_async_stream_response``, so its event grammar is + exercised through the same public seam a real stream takes: the SSE lines it + would have read, run through the adapter's own parsing. + """ + import asyncio + + class _FakeResponse: + status_code = 200 + + async def aiter_lines(self): + for event in events: + yield f"data: {json.dumps(event)}" + + async def aread(self): + return b"" + + class _FakeStreamCtx: + async def __aenter__(self): + return _FakeResponse() + + async def __aexit__(self, *exc): + return False + + class _FakeClient: + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + def stream(self, *args, **kwargs): + return _FakeStreamCtx() + + import httpx + + adapter = _oauth() + original = httpx.AsyncClient + httpx.AsyncClient = lambda *a, **k: _FakeClient() # noqa: E731 + try: + return asyncio.run( + adapter._async_stream_response("https://x", {}, {}) # noqa: SLF001 + ) + finally: + httpx.AsyncClient = original + + +def test_oauth_reader_merges_usage_so_both_counts_are_present(): + """AC-5: a stream whose message_start carries ``input_tokens`` and whose + message_delta carries only ``output_tokens`` harmonizes with BOTH present. + + Pre-session this leg ASSIGNED at message_delta, so the message-start figures + were lost — the prompt count would have been None here. + """ + raw = _drive_oauth_reader([_START, *_BODY, _DELTA_OUTPUT_ONLY]) + usage = _oauth().harmonize_response(raw, 0.0).metadata.usage + assert usage.prompt_tokens == 12, "the message_start input count was dropped" + assert usage.completion_tokens == 4, "the message_delta output count was dropped" + assert usage.cache_creation_input_tokens == 1564 + assert usage.full_prompt_tokens == 1576 + + +def test_oauth_reader_keeps_usage_when_the_stream_never_sends_message_delta(): + """The other half of the merge: an assign-at-delta reader reports nothing at + all for a stream that ends after prefill. Merging keeps what arrived.""" + raw = _drive_oauth_reader([_START, *_BODY]) + usage = _oauth().harmonize_response(raw, 0.0).metadata.usage + assert usage is not None + assert usage.prompt_tokens == 12 + assert usage.cache_creation_input_tokens == 1564 + + +# === AC-8 / AC-9 / AC-10 / AC-11 — the breakpoint placement ================== + + +def test_apikey_request_marks_the_last_block_of_the_last_message(): + """AC-8, string-content case: promoted to a one-element block list.""" + payload = _api().format_request_payload(MESSAGES) + content = payload["messages"][-1]["content"] + assert isinstance(content, list) and len(content) == 1 + assert content[0] == {"type": "text", "text": "hi", "cache_control": EPHEMERAL} + + +def test_apikey_request_marks_the_last_block_of_an_existing_block_list(): + """AC-8, block-list case: only the LAST block gets the marker.""" + payload = _api().format_request_payload([ + {"role": "user", "content": [ + {"type": "text", "text": "first"}, + {"type": "text", "text": "second"}, + ]}, + ]) + content = payload["messages"][-1]["content"] + assert "cache_control" not in content[0] + assert content[-1]["cache_control"] == EPHEMERAL + + +def test_oauth_request_marks_the_last_block_of_the_last_message(): + """AC-8 on the OAuth leg.""" + payload = _oauth().format_request_payload(MESSAGES) + content = payload["messages"][-1]["content"] + assert content[-1]["cache_control"] == EPHEMERAL + + +def test_bedrock_request_marks_the_tail_too(): + """The production leg inherits the placement.""" + adapter = BedrockAdapter(model_name="claude-opus-5", api_key="tok") + payload = adapter.format_request_payload(MESSAGES) + assert payload["messages"][-1]["content"][-1]["cache_control"] == EPHEMERAL + + +def test_the_marker_lands_on_a_tool_result_tail(): + """An agentic step ends on a tool result, so that is the block the breakpoint + must ride — the common case in production, not the text tail.""" + payload = _api().format_request_payload([ + {"role": "user", "content": "do it"}, + {"role": "assistant", "content": "", "tool_calls": [ + {"id": "c1", "function": {"name": "read_file", "arguments": "{}"}}, + ]}, + {"role": "tool", "tool_call_id": "c1", "content": "file body"}, + ]) + tail = payload["messages"][-1]["content"][-1] + assert tail["type"] == "tool_result" + assert tail["cache_control"] == EPHEMERAL + + +def test_apikey_system_is_the_array_form_and_the_text_is_byte_identical(): + """AC-9: array form (so the shape CAN carry a marker), same concatenated text + the pre-session bare-string form sent.""" + system_text = "You are Spren.\nAxis 1: …" + payload = _api().format_request_payload( + [{"role": "system", "content": system_text}, *MESSAGES] + ) + assert isinstance(payload["system"], list) + assert "".join(b["text"] for b in payload["system"]) == system_text + assert all(b["type"] == "text" for b in payload["system"]) + + +def test_apikey_payload_carries_exactly_one_marker(): + """AC-10, api-key leg: the tail marker only. No system marker this session — + the caller's system content is per-turn volatile, so a marker there would + write a fresh entry every call and read none.""" + payload = _api().format_request_payload( + [{"role": "system", "content": "sys"}, *MESSAGES] + ) + assert _markers(payload) == 1 + assert not any( + "cache_control" in b for b in payload["system"] if isinstance(b, dict) + ) + + +def test_oauth_payload_carries_exactly_two_markers(): + """AC-10, OAuth leg: the pre-existing static-prefix marker plus the new tail + one. Two is well under the API's ceiling of four.""" + payload = _oauth().format_request_payload( + [{"role": "system", "content": "sys"}, *MESSAGES] + ) + assert _markers(payload) == 2 + # And the first is still the Claude-Code prefix block, untouched. + assert payload["system"][0]["text"] == AnthropicOAuthAdapter.CLAUDE_CODE_PREFIX + assert payload["system"][0]["cache_control"] == EPHEMERAL + + +@pytest.mark.parametrize("build", ["api", "oauth"], ids=["apikey", "oauth"]) +def test_no_payload_ever_exceeds_four_markers(build): + """AC-10, the hard ceiling — asserted on a busy multi-block conversation.""" + messages = [{"role": "system", "content": "sys"}] + for i in range(6): + messages.append({"role": "user", "content": [ + {"type": "text", "text": f"ask {i}"}, + {"type": "text", "text": f"more {i}"}, + ]}) + messages.append({"role": "assistant", "content": f"reply {i}", "tool_calls": [ + {"id": f"c{i}", "function": {"name": "t", "arguments": "{}"}}, + ]}) + messages.append({"role": "tool", "tool_call_id": f"c{i}", "content": f"res {i}"}) + adapter = _api() if build == "api" else _oauth() + payload = adapter.format_request_payload(messages) + assert _markers(payload) <= 4 + # Exactly one breakpoint in the MESSAGES array, however long it is. + assert len(_marked_blocks({"messages": payload["messages"]})) == 1 + + +@pytest.mark.parametrize("build", ["api", "oauth"], ids=["apikey", "oauth"]) +def test_placement_is_deterministic_and_idempotent(build): + """AC-11: same input twice → byte-identical payload.""" + adapter = _api() if build == "api" else _oauth() + messages = [ + {"role": "system", "content": "sys"}, + {"role": "user", "content": "one"}, + {"role": "assistant", "content": "two"}, + {"role": "user", "content": [{"type": "text", "text": "three"}]}, + ] + first = adapter.format_request_payload([dict(m) for m in messages]) + second = adapter.format_request_payload([dict(m) for m in messages]) + assert json.dumps(first, sort_keys=True) == json.dumps(second, sort_keys=True) + + +def test_a_block_that_already_carries_a_marker_never_gets_a_second_one(): + """AC-11's other half — asserted on the helper directly, so the guard is + pinned regardless of which adapter calls it.""" + messages = [{"role": "user", "content": [ + {"type": "text", "text": "a", "cache_control": EPHEMERAL}, + {"type": "text", "text": "b"}, + ]}] + mark_conversation_tail_for_cache(messages) + content = messages[-1]["content"] + assert sum(1 for b in content if "cache_control" in b) == 1 + assert "cache_control" not in content[-1] + + +def test_marking_never_mutates_the_callers_block_dicts(): + """The durable conversation shares these dicts, so a marker stamped in place + would leak into the persisted JSONL on the next rewrite.""" + block = {"type": "text", "text": "hi"} + messages = [{"role": "user", "content": [block]}] + mark_conversation_tail_for_cache(messages) + assert block == {"type": "text", "text": "hi"}, "the caller's block was mutated" + assert messages[-1]["content"][-1]["cache_control"] == EPHEMERAL + + +@pytest.mark.parametrize( + "messages", + [ + [], + [{"role": "user", "content": ""}], + [{"role": "user", "content": []}], + [{"role": "user", "content": [{"type": "server_tool_use", "id": "x"}]}], + ], + ids=["empty-list", "empty-string", "empty-content", "unmarkable-block-type"], +) +def test_nothing_safe_to_mark_is_a_no_op(messages): + """A marker on a block type the API does not accept it on is a 400 — a missed + cache entry only costs money, so the unrecognized tail is skipped.""" + before = json.dumps(messages, sort_keys=True) + mark_conversation_tail_for_cache(messages) + assert json.dumps(messages, sort_keys=True) == before + + +def test_the_marker_lands_after_the_json_mode_hint_not_before_it(): + """The json-mode fallback appends a hint block to the tail message, so the + breakpoint must be placed after it or it stops being the tail.""" + payload = _api().format_request_payload(MESSAGES, json_mode=True) + content = payload["messages"][-1]["content"] + assert "JSON" in json.dumps(content) + assert content[-1]["cache_control"] == EPHEMERAL + assert sum(1 for b in content if "cache_control" in b) == 1