Merge pull request #46 from rezaho/feature/specialized-agent-serialization

feat(framework): specialized-agent kind/params serialization (S09)

Author: rezaho

docs/architecture/framework/decisions/ADR-009-specialized-agent-serialization.md

@@ -0,0 +1,70 @@
+# ADR-009: Specialized-Agent Serialization (`kind`/`params` AgentSpec contract)
+
+**Status**: Proposed — design forks selected with the user 2026-05-17 (Session 09 Phase A); awaiting written ADR approval (escalation gate: public wire-contract change + `WIRE_SCHEMA_VERSION` bump).
+**Date**: 2026-05-17
+**Implements**: Framework Session 09 — `docs/implementation/framework/sessions/v0.3.0/09-specialized-agent-serialization.md`
+**Related**: ADR-008 (unified `NodeKind` model — this applies the same discriminator+registry+behaviour-in-class principle to the agent axis; **supersedes ADR-008 Decision 9's literal `agent_type` wording with `kind`**); Session 10 / ADR-010 (`10-runtime-credential-resolution.md` — later collapses the `runnable`/dual-`ModelConfig` path; this ADR's secrets decision is deliberately stable across it).
+
+## Context
+
+The canonical wire serializer flattens every specialized `Agent` subclass to base `Agent` on round-trip. Confirmed from primary source: `agent_to_pydantic` admits all 5 subclasses through `isinstance(agent, Agent)` (`agents/serialize.py:85`) but extracts only the base surface (`:110-125`); `pydantic_to_agents` unconditionally rebuilds a base `Agent(...)` (`:158`); `AgentSpec` has no discriminator (`:50-68`). `agents/serialize.py` is the only agent serializer (no subclass dispatch anywhere). A workflow authored with `WebSearchAgent`/`BrowserAgent`/`CodeExecutionAgent`/`DataAnalysisAgent`/`FileOperationAgent` cannot be saved and reloaded as that subclass — a user-stated requirement gap. ADR-008 Decision 9 / Alternative 6 (`ADR-008:75,96,104`) deliberately deferred this to Session 09 with the principle: *a typed graph entity round-trips via a closed wire discriminator + a framework-side class registry; behaviour lives in the class; bindings are injected at hydrate*.
+
+The framework already ships every pattern this needs: the closed discriminator + single-source registry + behaviour-in-class (`det_nodes.py:188-209` `NODE_KIND_BEHAVIOUR`/`RESERVED_NAME_TO_KIND`/`RESERVED_NAME`); the curated typed wire mirror that drops secrets (`models/serialize.py:50-75` `ModelConfigSpec` drops `api_key`); name-only-on-wire + inject-at-hydrate (`tool_registry`/`handler_registry`, `agents/serialize.py:130`, `topology/serialize.py:474`); async construction via `if hasattr(cls,'create_safe'): await cls.create_safe(...) else cls(...)` (`pool_factory.py:44-49`, `agent_pool.py:156,209`). This ADR composes them; it invents no new mechanism.
+
+The constructor reality (primary source) forces three non-obvious decisions: specialized `__init__`s take non-JSON objects (`CodeExecutionConfig`, `RunFileSystem`, `ModelConfig`) and runtime-derived defaults (`base_directory or Path.cwd()` — `code_execution_agent.py:78`); `WebSearchAgent` takes four credential kwargs (`web_search_agent.py:28-31`); `BrowserAgent` is only fully constructible via the **async** `create_safe` (`browser_agent.py:1085,1126,1166`) — the sync `__init__` yields a browser-unready instance.
+
+## Premise Ledger
+
+| # | Premise | Source | Status |
+|---|---------|--------|--------|
+| P1 | Serializer flattens all 5 subclasses to base `Agent` | primary-source | CONFIRMED@agents/serialize.py:85,110-125,158 (sole agent serializer) |
+| P2 | ADR-008 blesses the discriminator+registry+behaviour-in-class principle for the agent axis as Session 09 | primary-source | CONFIRMED@ADR-008:75,96,104 |
+| P3 | det_nodes is a faithfully transferable single-source-registry precedent | primary-source | CONFIRMED@det_nodes.py:79,111,139,188-209 |
+| P4 | `marsys.agents.__all__` is the package public API, not the specialized set | primary-source | CONFIRMED@agents/__init__.py:32-64 (lists BaseAgent/Agent/AgentPool/LearnableAgent/memory/registry/utils) |
+| P5 | Specialized `__init__`s carry non-JSON objects + runtime-derived defaults + secrets | primary-source | CONFIRMED@code_execution_agent.py:49-50,78; data_analysis_agent.py:50-51,75; file_operation_agent.py:38; browser_agent.py:807,819,1100; web_search_agent.py:28-33 |
+| P6 (load-bearing) | `BrowserAgent` is only fully constructible async; sync `__init__` yields a browser-unready instance | primary-source | CONFIRMED@browser_agent.py:1085 (`async create_safe`), :1126 (sync `cls(...)`), :1166 (`await _initialize_browser()` is the async-only work) |
+| P7 | A field named exactly `model` is Pydantic-v2-clean; `model_config` is impossible (the config attr); only the `model_` *prefix* is protected | primary-source | CONFIRMED@empirical test pydantic 2.13.3 (field `model` → 0 warnings; `model_config` → `PydanticUserError`); `serialize.py:53` already uses `model_config = ConfigDict(...)` |
+| P8 | `LearnableAgent`/`BaseLearnableAgent` are outside the `Agent` subtree; `agent_to_pydantic` `TypeError`s on them today (not a silent flatten) | primary-source | CONFIRMED@learnable_agents.py:35,227; agents/serialize.py:85-91 |
+| P9 | The framework's own async-construction idiom is `hasattr(cls,'create_safe')` dispatch | primary-source | CONFIRMED@pool_factory.py:44-49, agent_pool.py:156,209 |
+| P10 | Hydration is caller-side, before the already-async `Orchestra.run`; the only in-repo sync callers are the S08 example + ~6 test sites | primary-source | CONFIRMED@orchestra.py:733 (`async def run`); examples/session08_workflow_roundtrip.py:156; test_serialize.py:86,113,151,317,331; test_s08_coverage_gaps.py:85 |
+| P11 | `WIRE_SCHEMA_VERSION = 2`; an additive-discriminator change is the same class S08 used for 1→2 | primary-source | CONFIRMED@topology/serialize.py:81,588 |
+
+**Frame check.** Most load-bearing: P2 (the principle is ADR-blessed for this axis) and P6 (async is intrinsic, not incidental). Falsifier sought for the frame "add `kind`+`params`+registry, behaviour in class": that a first-class serializer already exists (none — P1, sole serializer) or that the pattern fights an invariant (it does not — it is the literal `det_nodes` precedent, P3). Falsifier sought for the async decision: that a sync hydrate can fully build a `BrowserAgent` (refuted — P6). Frame holds; the only sharp correction vs the Session-09 draft was that `params` is a *curated typed subset* (P5), not raw constructor kwargs, and the registry is a dedicated dict (P4), not `__all__`.
+
+## Decision
+
+**Y-statement.** In the context of the canonical wire format flattening specialized `Agent` subclasses to base `Agent`, facing constructors that carry non-JSON objects, secrets, and async-only construction, we decided to **add an additive `kind` discriminator + a typed per-subclass `*ParamsSpec` to `AgentSpec`, resolved through one dedicated closed `AGENT_KIND_REGISTRY` (the `NODE_KIND_BEHAVIOUR` analog), with subclass behaviour rebuilt in the class on hydrate via a single always-async hydrate entrypoint** — and neither raw-kwarg `params`, nor an `__all__`/reflection-derived registry, nor a sync/dual hydrate API — to achieve lossless specialized-agent round-trip that is honest about its async nature and leaks no secrets, accepting a `WIRE_SCHEMA_VERSION` 2→3 break (one in-repo mirror consumer, contracted follow-up), an `agent_model`→`model` rename in the same break, and migration of the in-repo sync hydrate callers.
+
+1. **`AgentSpec.kind: str = "agent"` + `AgentSpec.params`, additive.** Default `"agent"` → base `Agent`, existing specs byte-identical, no migration. `kind`/`params` are additive on the existing flat surface — every base field still round-trips for a subclass. `kind` mirrors `NodeSpec.kind` (`topology/serialize.py:131`). **This supersedes ADR-008 Decision 9's literal `agent_type` wording** (recorded user decision: one discriminator word across the wire; not "type").
+2. **`AGENT_KIND_REGISTRY` is the single source.** A dedicated module-level `Dict[str, Type[Agent]]` co-located with the subclasses, the exact `NODE_KIND_BEHAVIOUR` analog. Each subclass declares `WIRE_KIND: ClassVar[str]` (the `RESERVED_NAME` analog); the reverse `class→kind` map is **derived by inversion**, never hand-maintained twice. A CI test cloned from `tests/coordination/topology/test_node_kind_registry.py` asserts membership and bidirectional `WIRE_KIND`↔registry consistency — a forgotten agent fails the build. **Not** `__all__` (P4), **not** `Agent.__subclasses__()`/reflection (RCE-class risk instantiating an arbitrary class named in stored JSON; import-order fragile). Extension = one `WIRE_KIND` + one entry, no dispatch edits.
+3. **`params` = typed per-subclass `*ParamsSpec`, curated.** Each a hand-picked declarative subset (the `ModelConfigSpec`/base-`AgentSpec` convention). Non-JSON / runtime-derived kwargs (`filesystem`, `code_config`, `root_dir`) are not on the wire; the subclass `__init__` reconstructs them from declarative inputs on hydrate as it does today. Not an open `dict`.
+4. **Single always-async hydrate entrypoint (B′).** `pydantic_to_agents`/`pydantic_to_topology` become `async`, dispatching via `AGENT_KIND_REGISTRY` with the `hasattr(cls,'create_safe')` idiom (P9). Hydration is intrinsically async (P6); the honest type is `async`. In-repo sync callers (P10) migrate to `await`/`asyncio.run`.
+5. **Secrets dropped from the wire by construction.** No credential field in any `*ParamsSpec` (the `ModelConfigSpec`-drops-`api_key` precedent). The subclass `__init__`'s existing env-var resolution is the resolver, unchanged; an injected secrets map may be threaded at the async entrypoint mirroring `tool_registry`/`handler_registry`. This references nothing from the Session-07 `runnable` mechanism, so it is **stable across Session 10 / ADR-010**.
+6. **`agent_model` → `model`, bundled.** Verified clean (P7); one `WIRE_SCHEMA_VERSION` 2→3 break carries both the discriminator and the rename. CHANGELOG + DEPRECATIONS note it.
+7. **`LearnableAgent`/`BaseLearnableAgent` explicitly out of scope.** Outside the `Agent` subtree (P8); including them changes the `isinstance` contract and needs a `BaseAgent`-level path + their missing `_model_config` — strictly larger than the reported gap. They remain unserializable exactly as today, documented (not a silent drop). User decision 2026-05-17.
+
+### Scope boundaries
+
+**In**: Decisions 1–7 for the 5 `Agent`-subtree specialized agents; registry + CI test; `*ParamsSpec`; async hydrate + in-repo caller migration; schema 2→3; ADR-009; CHANGELOG/DEPRECATIONS; a runnable round-trip example.
+**Out**: `LearnableAgent` (Decision 7); Spren mirror update + stored-data migration (contracted Spren-side follow-up); Session-10 credential/`runnable` collapse; new specialized classes; edge/pattern semantics; v0.4 shim removal.
+
+## Consequences
+
+- **Positive**: specialized agents losslessly round-trip as their subclass; one single-sourced agent taxonomy mirroring the node axis; the hydrate API is honest about being async and structurally cannot mis-route on unparsed contents; no secret and no machine-specific path ever reaches the wire; adding a specialized agent is one declaration.
+- **Negative**: `WIRE_SCHEMA_VERSION` 2→3 + `agent_model`→`model` break the wire contract → Spren mirror + suite diverge until its contracted follow-up; every in-repo sync hydrate caller migrates to async; `LearnableAgent` stays unserializable (explicit, documented).
+- **Recorded limitation**: hydrate synthesizes the constructor call and filters kwargs to the callee's signature (`_accepted_kwargs`). A fixed-signature specialized factory receives only the base-surface kwargs it declares; a base `AgentSpec` field it does not expose is not round-tripped for that subclass. Concretely, `BrowserAgent` does not carry `bidirectional_peers` — neither `BrowserAgent.__init__` nor `create_safe` accepts it, so it cannot be set non-default through the class in the first place. This is a property of that class's own constructor contract, surfaced here, not a silent serializer drop. The sync subclasses (which take `**kwargs`) round-trip the full base surface.
+- **Risks**: a subclass routed through a `params`-only path silently regressing the base surface → explicit additive-round-trip acceptance criterion + characterization regression pin; a `*ParamsSpec` leaking a non-JSON/secret field → per-subclass characterization from real `__init__` + a secrets-off-wire test; a missed sync caller → grep-confirmed caller set + full-suite green vs S08 baseline; BrowserAgent hydrating browser-unready → an explicit async-path test asserting browser-readiness.
+
+## Alternatives Rejected
+
+1. **Dual sync+async hydrate API (option A).** Forces the caller to pick the entrypoint by workflow contents they have not parsed yet; the sync path hard-errors on a valid `BrowserAgent` workflow. Rejected: inelegant contract; B′ is one always-correct entrypoint.
+2. **Single async + faked sync wrapper (option B).** The wrapper hides `asyncio.run` → `RuntimeError` whenever an event loop is already running (the dominant hydrate-then-`await run` case). "Fixes" are `nest_asyncio` (global monkeypatch imposed on consumers) or thread-offload (blocks the consumer loop through a real browser launch). Rejected: known anti-pattern, fixes worse than the disease.
+3. **Defer construction into `Orchestra.run` (option C).** Breaks S08's fully-materialized-`Topology` contract and pushes hydration into the TRUNK-CRITICAL orchestrator ADR-008 deliberately kept untouched. Rejected: largest blast radius.
+4. **Open validated `dict` for `params`.** Special-cases secrets/`Path` per subclass at validation anyway — same work, less safety, breaks the "every wire object is a typed mirror" convention, defers the break to a user's load. Rejected.
+5. **Registry single-sourced from `__all__` / `Agent.__subclasses__()`.** `__all__` is the package API (P4); reflection is an RCE-class risk and import-order fragile. Rejected: dedicated dict, det_nodes-style.
+6. **Bundle `LearnableAgent`.** Changes the `isinstance(agent, Agent)` contract + needs a `BaseAgent`-level path + their missing `_model_config`. Rejected: strictly larger than the reported gap; explicit out-of-scope with documentation (not a silent drop).
+7. **Keep `agent_model`.** The Pydantic-namespace justification was over-cautious (P7). Rejected: the schema is breaking for the discriminator anyway — one break, honest name.
+
+## Migration & backward-compat
+
+`kind="agent"` default ⇒ every existing v2 `AgentSpec` is byte-identical in behaviour; no stored-spec migration for base agents. The wire is breaking for `agent_model`→`model` and the schema-version 2→3 — the version boundary is the contract (no dual-field shim — anti-pattern: legacy retention). Framework ships the new shape; Spren migrates its mirror + store in its own contracted PR. In-repo sync hydrate callers move to async in this session.