Skip to content

feat(tools): implement extended tool definitions with native Pydantic… - #2

Closed
Leoyzen wants to merge 371 commits into
mainfrom
feature/extended_tool_integration
Closed

feat(tools): implement extended tool definitions with native Pydantic…#2
Leoyzen wants to merge 371 commits into
mainfrom
feature/extended_tool_integration

Conversation

@Leoyzen

@Leoyzen Leoyzen commented Feb 9, 2026

Copy link
Copy Markdown
Collaborator

…AI integration

  • Introduce support for the 'prepare' protocol and explicit 'function_schema' overrides in tool definitions
  • Utilize native pydantic_ai.Tool.from_schema to enable full validation capabilities, including validate_json
  • Implement an automated schema generation fallback using schemez to handle complex context types (AgentContext, RunContext)
  • Enhance Tool.to_pydantic_ai to handle custom JSON schemas and context injection natively
  • Ensure seamless integration with agent-level tool wrapping for confirmation and execution hooks
  • Provide comprehensive test coverage for dynamic schema generation and validation paths

@gemini-code-assist

Copy link
Copy Markdown

Summary of Changes

Hello @Leoyzen, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly upgrades the tool definition and integration capabilities within the system, focusing on a more robust and flexible interaction with the pydantic-ai library. By introducing explicit schema overrides, a 'prepare' protocol, and intelligent fallback for schema generation, it ensures that tools, especially those dealing with complex context objects, are correctly validated and seamlessly integrated into the agent's workflow. The changes aim to provide developers with greater control over tool behavior and schema definition, leading to more reliable and maintainable agent tools.

Highlights

  • Extended Tool Definitions: Introduced support for the 'prepare' protocol and explicit 'function_schema' overrides within tool definitions, allowing for more flexible and customizable tool behavior.
  • Native Pydantic-AI Integration: Leveraged pydantic_ai.tools.Tool.from_schema to enable full validation capabilities, including validate_json, for tool arguments, enhancing robustness and type safety.
  • Automated Schema Generation Fallback: Implemented an automated fallback mechanism using schemez for schema generation when pydantic-ai's native schema generation fails, particularly for complex context types like AgentContext and RunContext.
  • Enhanced Tool Conversion: The Tool.to_pydantic_ai method has been significantly enhanced to natively handle custom JSON schemas and context injection, streamlining the integration of custom tools with the pydantic-ai framework.
  • Improved Context Handling: Refined how AgentContext and RunContext are passed to and accessed by tool functions, ensuring seamless context injection and data retrieval within the agent's operational flow.
  • Comprehensive Test Coverage: Added a new, extensive test suite (tests/tools/test_tool_schema.py) to cover dynamic schema generation, validation paths, fallback mechanisms, and various tool property behaviors.
Changelog
  • src/agentpool/agents/native_agent/agent.py
    • Updated imports to reflect changes in pydantic_ai and AgentContext usage.
    • Modified get_agentlet to correctly type PydanticAgent with AgentContext[TDeps] and to dynamically wrap tools before passing them to the PydanticAgent constructor.
    • Added a new private method _process_node_stream to centralize the processing of node stream events, including file tracking and tool event handling.
    • Adjusted _stream_events to create and pass an AgentContext instance as dependencies to the agentlet.iter call.
  • src/agentpool/tools/base.py
    • Added prepare and function_schema fields to the Tool class for extended customization.
    • Introduced _get_effective_prepare, _detect_takes_ctx, and _get_json_schema helper methods to manage tool preparation, context detection, and JSON schema generation.
    • Refactored to_pydantic_ai to conditionally use pydantic_ai.tools.Tool.from_schema when custom JSON schemas are provided or generated, ensuring proper validation and validate_json support.
    • Implemented a fallback mechanism in _get_json_schema to use schemez for schema generation if pydantic-ai's native method fails, particularly for complex types like AgentContext.
    • Updated from_callable methods to accept and propagate the new prepare and function_schema parameters.
  • src/agentpool_config/tools.py
    • Added prepare, function_schema, and schema_override fields to BaseToolConfig to allow configuration of these new tool properties.
    • Modified get_tool to load the prepare callable from an import string and pass all new configuration parameters to Tool.from_callable.
  • tests/test_schema_override.py
    • Removed unused ToolDefinition import.
    • Updated assertions to verify that schema_override is correctly integrated into the function_schema's json_schema rather than relying on a prepare function for schema modification.
  • tests/tools/test_runcontext.py
    • Adjusted type hints for RunContext in data_with_run_ctx to be more specific (RunContext[AgentContext[dict[str, str]]]).
    • Corrected data access logic in data_with_run_ctx and data_with_agent_ctx to properly retrieve user data from the context objects.
  • tests/tools/test_tool_schema.py
    • Added a new test file to provide comprehensive coverage for tool schema generation, including fallback scenarios, validate_json presence, native RunContext handling, schema overrides, and various tool properties and execution paths.
Activity
  • The pull request introduces new features and refactors existing code to improve tool integration and schema handling.
  • New test cases have been added to ensure the correctness and robustness of the implemented changes.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a significant and well-designed refactoring of tool definitions and their integration with pydantic-ai. The new Tool.to_pydantic_ai method with its schema generation logic, including the fallback to schemez for complex types like AgentContext, is a robust solution. The introduction of prepare and function_schema overrides provides valuable flexibility. The comprehensive new test suite in test_tool_schema.py is excellent and ensures the new functionality is well-covered. I've identified a couple of issues related to leftover code from the refactoring that should be addressed.

Comment thread src/agentpool/agents/native_agent/agent.py
Comment thread src/agentpool/agents/native_agent/agent.py Outdated
@Leoyzen

Leoyzen commented Feb 9, 2026

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a significant and well-thought-out refactoring of tool definition and schema handling. The move to utilize pydantic_ai.Tool.from_schema and implementing a fallback mechanism with schemez for complex types is a great improvement for robustness. The new test suite for schema generation is comprehensive and adds a lot of value.

I have two main points of feedback:

  1. There's an inconsistency in how schema_override is applied between the primary schema generation path and the schemez fallback path. The fallback path misses parameter-level overrides.
  2. The dependency injection for tools requesting AgentContext seems to create a nested context structure, which complicates tool implementations as seen in the updated test.

Overall, these are excellent changes that improve the tool integration architecture. Addressing these two points will further enhance consistency and maintainability.

Comment thread src/agentpool/tools/base.py Outdated
Comment thread tests/tools/test_runcontext.py Outdated
@Leoyzen

Leoyzen commented Feb 9, 2026

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a major refactoring of tool definitions to better integrate with pydantic-ai. The changes enable native schema generation and validation by using pydantic-ai's Tool.from_schema, with a fallback to schemez for complex types like AgentContext. This is a significant improvement that enhances validation capabilities. The implementation is well-tested with a comprehensive new test suite. My review includes a couple of suggestions for improving type safety and code style.

deps_type=self.deps_type or NoneType,
output_type=final_type,
deps_type=AgentContext[TDeps],
output_type=cast(Any, final_type),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The use of cast(Any, final_type) obscures the type information and suppresses potential type errors. It would be more maintainable to resolve the underlying type mismatch. If the cast is unavoidable due to a limitation in a dependency's typing, please add a comment explaining why it's necessary.


# Return only the parameters part (the "object" schema)
# Use model_dump - schemez.FunctionSchema has this method (pydantic-compatible)
schema_dump = getattr(schema, "model_dump")() # noqa: B009, type: ignore[attr-defined]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using getattr with a constant string like "model_dump" is generally discouraged and flagged by linters (e.g., flake8-bugbear B009). Since schema is an instance of schemez.FunctionSchema (a Pydantic model), you can call model_dump() directly for cleaner code.

            schema_dump = schema.model_dump()  # type: ignore[attr-defined]

@Leoyzen
Leoyzen force-pushed the feature/extended_tool_integration branch from 048333c to 767333b Compare February 9, 2026 07:17
@Leoyzen Leoyzen closed this Apr 30, 2026
Leoyzen added a commit that referenced this pull request May 27, 2026
Add fork_session parameter to AgentCapabilities.create() and SessionCapabilities construction. Also add parameter to InitializeResponse.create() and enable it in the ACP server.

Previously, session/fork was fully implemented but the capability could never be advertised to clients because AgentCapabilities.create() had no fork_session parameter.
Leoyzen added a commit that referenced this pull request Jun 28, 2026
1. [Not adopted] base_agent.py:559 — StreamCompleteEvent/RunErrorEvent are
   already imported at runtime (line 23). Python 3.13+ supports isinstance
   with union types. Gemini misidentified the import as TYPE_CHECKING-only.
2. [HIGH] agent.py:967 — _execute_node now checks RunErrorEvent before
   accessing turn.final_message (prevents RuntimeError masking original error)
3. [HIGH] agent.py:1023 — _stream_events same fix as #2
4. [MEDIUM] core.py:1496 — _start_run_handle keeps strong reference to
   background task in _background_tasks set (prevents GC destruction)
5. [LOW] acp_agent/turn.py:111 — removed unused _initial_message_history
   attribute (dead code)

Tests: 5 new tests in test_pr64_round5_fixes.py, all 38 PR64 tests pass.
Leoyzen added a commit that referenced this pull request Jun 28, 2026
1. [Not adopted] base_agent.py:559 — StreamCompleteEvent/RunErrorEvent are
   already imported at runtime (line 23). Python 3.13+ supports isinstance
   with union types. Gemini misidentified the import as TYPE_CHECKING-only.
2. [HIGH] agent.py:967 — _execute_node now checks RunErrorEvent before
   accessing turn.final_message (prevents RuntimeError masking original error)
3. [HIGH] agent.py:1023 — _stream_events same fix as #2
4. [MEDIUM] core.py:1496 — _start_run_handle keeps strong reference to
   background task in _background_tasks set (prevents GC destruction)
5. [LOW] acp_agent/turn.py:111 — removed unused _initial_message_history
   attribute (dead code)

Tests: 5 new tests in test_pr64_round5_fixes.py, all 38 PR64 tests pass.
Million-mo pushed a commit that referenced this pull request Jul 3, 2026
…til, remove dead code

- Extract _wrap_for_pydantic_ai into shared tool_wrapping.py module
- StaticToolsetFactory now calls wrap_tool_for_pydantic_ai directly,
  removing tight coupling to deprecated ResourceProvider (comment #1)
- ResourceProvider._wrap_for_pydantic_ai delegates to shared util
  for backwards compatibility
- Remove redundant 'if not toolsets:' dead code (comment #2)
- Fix return type: AbstractToolset[Any] | None (was AbstractCapability)
- Clean up unused imports (inspect, ModelRetry) from base.py
- Rebase onto latest refactor/thin-wrapper (includes PR #97)
Leoyzen added a commit that referenced this pull request Jul 5, 2026
Design correction:
- handle_elicitation() now raises CallDeferred directly (not side-channel)
- MCP elicitation_handler catches CallDeferred, converts to side-channel
  (FastMCP workaround isolated to MCP client only)
- Local tools like question_for_user need zero adaptation

PR review fixes:
- #1: Accumulate all pending_calls, single checkpoint after loop
  (prevents overwrite when multiple parallel elicitation calls)
- #2: Bypass SessionBusyError for in-process elicitation resume
  (add allow_active_run parameter to _with_resume_lock)

CI fixes:
- ruff: D202 blank line after docstring, D104 missing package docstring
- ruff format: 3 files reformatted
- snapshot: update ACP event converter snapshot (pre-existing drift)

Refs: #107
Leoyzen pushed a commit that referenced this pull request Jul 6, 2026
…74) (#93)

* refactor(orchestrator): split core.py into event_bus, session_controller, session_pool

Phase 1 of thin-wrapper refactor (#74): Split the 3035-line
orchestrator/core.py into three focused modules:

- event_bus.py (595 lines): EventBus, EventEnvelope, drain_and_merge,
  and all merge helper functions
- session_controller.py (1382 lines): SessionController, SessionState,
  exceptions, SessionLifecyclePolicy
- session_pool.py (1145 lines): SessionPool

core.py reduced to thin re-exports for backward compatibility.
All 460 orchestrator tests pass, mypy clean, ruff clean.

Also includes OpenSpec change artifacts (proposal, design, specs, tasks)
for the full 8-phase thin-wrapper refactor plan.

* test(agents): verify node-level Capability hooks on standalone run path (#95)

* fix(orchestrator): address 8 Gemini Code Assist review comments on PR #93

- start_cleanup_task: start _start_cleanup_loop + strong task refs in _background_tasks set
- get_or_create_session_agent: validate session_id non-empty before proceeding
- create_team_from_config: use stateless cfg.get_agent() instead of session-bound agent (fixes MCP subprocess leak)
- _close_session_unlocked: wrap child session close in try-except (cascade resilience)
- close_session: same try-except for second child close loop
- event_bus _drain_dead_streams: catch all exceptions, not just anyio-specific ones
- event_bus close_session: same broad exception handling for send_stream aclose
- session_pool close_session: wrap in try-finally so EventBus + cache cleanup always runs

460 orchestrator tests pass, ruff + mypy clean.

* chore: limit pre-commit pytest to orchestrator subset (full suite in CI)

* refactor(agents): Phase 2 — simplify run_stream producer/consumer to direct delegation

Remove the redundant producer/consumer pattern from BaseAgent.run_stream()
Path B. Previously, _run_stream_once() ran in an asyncio.ensure_future
producer task that published events to EventBus, then the consumer drained
them via drain_and_merge(). Now run_stream() directly iterates
_run_stream_once() with inline event handler dispatch.

Key finding: NativeTurn.execute() already calls agent_run.next(node)
(line 206 of turn.py), so pdai Capability hooks were already firing on
all paths. The producer/consumer was redundant indirection, not a
hook-breaking bug.

Changes:
- BaseAgent.run_stream(): remove _producer task + drain_and_merge consumer,
  replace with direct async for on _run_stream_once()
- Event handler dispatch moved inline (was in consumer loop)
- EventBus cleanup preserved (close_session on local bus)
- New test: test_capability_hooks_standalone.py verifies wrap_run hook
  fires on standalone run_stream() path

Verification: 837 tests pass (agents + orchestrator), ruff clean.

* fix(orchestrator): restore producer/consumer pattern + fix ruff lint

Phase 2's direct delegation removed EventBus subscription, losing events
that bypass _stream_events() (ToolCallProgressEvent, SpawnSessionStart).
Restore producer/consumer with asyncio.Queue-compatible drain_and_merge.

Also fix:
- F401: remove unused EventBus import in _stream_events()
- I001: fix import sorting in test_capability_hooks_standalone.py

Fixes CI failures:
- test_workers_child_session_persisted_with_correct_parent (SpawnSessionStart)
- test_progress_handler_with_agent_non_streaming/streaming (progress events)
- test_agent_stream_progress_events (ToolCallProgressEvent)
- Lint (ruff check) failures

* test(agents): verify node-level Capability hooks on standalone run path

Address Gemini Code Assist review on PR #95: the previous test only
verified wrap_run (a run-level hook triggered by agentlet.iter()),
which would fire even on the legacy path. Phase 2's core invariant is
that agent_run.next(node) is called on the standalone path, triggering
node-level hooks.

Expand HookTrackerCapability to implement and track:
- wrap_node_run
- before_model_request
- after_node_run (with correct signature including node param)

Rename test to test_capability_hooks_fire_on_standalone_run to reflect
that it verifies all four hooks, not just wrap_run.

Verification: 1 test passes, ruff + mypy clean.

---------

Co-authored-by: Test <test@test.com>

* refactor(orchestrator): Phase 3 — migrate EventBus to asyncio.Queue (#96)

* refactor(orchestrator): Phase 3 — migrate EventBus from anyio streams to asyncio.Queue

Replace anyio memory object streams with asyncio.Queue throughout the
EventBus and all consumers. Introduces configurable overflow policies
(drop_oldest, drop_newest, drop_subscriber) replacing the previous
hybrid timeout→drop strategy.

Production changes:
- EventBus: anyio.Lock → asyncio.Lock, memory streams → asyncio.Queue
- EventBus: new _enqueue() method with overflow policy dispatch
- EventBus: drain_and_merge() now drains asyncio.Queue (QueueShutDown/QueueEmpty)
- session_pool.py: stream.receive()→queue.get(), receive_nowait()→get_nowait()
- session_pool.py: anyio.EndOfStream→asyncio.QueueShutDown, WouldBlock→QueueEmpty
- mixins.py: _consumer_streams type annotation anyio.abc→asyncio.Queue

Test changes:
- All tests using 'async for event in queue' replaced with queue.get() loop
- gen.shutdown()→gen.aclose() (async generators have no shutdown())
- anyio.fail_after() used instead of asyncio.timeout() (anyio pytest plugin)
- _stream_empty via statistics()→queue.empty()
- test_event_bus_backpressure.py: rewritten for asyncio.Queue API
- test_replay_buffer_turn_isolation.py: rewritten for queue.get()
- test_sessionpool_reasoning_redflag.py: _drain_queue() helper

Verification:
- 460 orchestrator tests pass (0 failures)
- ruff: 0 errors

* fix(orchestrator): complete asyncio.Queue migration + adopt Gemini review on PR #96

Fixes all CI failures from Phase 3 (mypy, unit, integration, core tests):

Production fixes:
- acp_agent.py: migrate bus_stream from anyio stream API (.receive()/
  .receive_nowait()) to asyncio.Queue API (.get()/.get_nowait()), add
  QueueShutDown handling, fix type annotation, remove unused import
- event_bus.py: fix mypy comparison-overlap error in _enqueue overflow
  policy dispatch (check 'block' before membership test)
- mixins.py: fix consumer task lifecycle (await task exit before cleanup,
  suppress exceptions during unsubscribe), add contextlib import
- message_routes.py + session_pool_integration.py: wrap raw queue iteration
  in drain_and_merge() (Queue has no async-for protocol)

Test fixes (Gemini Code Assist review):
- test_performance.py + test_resume_session.py: replace destructive
  _stream_empty (get_nowait consumes items) with non-destructive
  queue.empty()
- test_event_bus.py: test_close_session_signals_end_of_stream now drains
  queue in a loop until QueueShutDown (was only calling get_nowait once)
- test_event_bus_backpressure.py: rename misleading test to
  test_backpressure_retains_subscriber_with_drop_oldest (default policy
  is drop_oldest, subscriber is NOT dropped)
- All consumer tests: migrate from anyio stream API to asyncio.Queue API
  (get/get_nowait/QueueShutDown/QueueEmpty)
- mock_stream.py: update helper to use asyncio.Queue instead of anyio streams

Verification: 1244 tests pass, mypy clean, ruff clean.

---------

Co-authored-by: Test <test@test.com>

* feat(config): Phase 4 — build graph_translation.py + extend GraphStepConfig

- Create src/agentpool_config/graph_translation.py with translate_team_to_graph,
  translate_teams_to_graphs, translate_connections_to_edges,
  translate_config_to_graph, build_steps_from_agents
- Extend GraphStepConfig with team fields: shared_prompt, prompt_template,
  member_timeout, member_retry_attempts, member_retry_delay
- Export all new types and functions from agentpool_config.__init__
- 22 unit tests covering all translation paths

* feat(manifest): Phase 4 Part 2 — auto-translate teams: to graph: in manifest

- Add explicit graph: GraphConfig | None field to AgentsManifest
- Add _auto_translate_teams_to_graph model validator that runs
  after _populate_node_names: when graph is None, translates
  teams: and connections: into a unified GraphConfig
- teams: mode=sequential produces chained steps (start->s1->s2->end)
- teams: mode=parallel produces Fork+Join (start->[all], [all]->end)
- Existing graph: section takes precedence (no translation)
- 482 orchestrator+config tests pass, ruff clean

* deprecate(teams): mark TeamConfig.get_team() as deprecated

Add DeprecationWarning directing users to
translate_team_to_graph() from agentpool_config.graph_translation.
Full Team/TeamRun removal requires restoring pool-level graph
execution — deferred to a later phase.

* fix(config): address Gemini review on PR #97 — connection translation + openspec sync

- Fix KeyError: skip FileConnectionConfig/CallableConnectionConfig in
  translate_connections_to_edges (only NodeConnectionConfig has 'name')
- Fix implicit truthiness on teams dict (use 'is not None' per review)
- Simplify downstream conditional (remove redundant 'and teams is not None')
- Add 6 unit tests: node→edge, file skip, callable skip, mixed, full
  config translation, empty teams dict
- Update openspec tasks.md: mark Phase 4 tasks 4.1-4.9 as completed
- Update openspec spec.md: add 'Agent connections translated to graph
  edges' requirement with skip-scenarios for file/callable connections

* feat(tools): Phase 5 — ToolsetFactory protocol replacing ResourceProvider

- Define ToolsetFactory as runtime_checkable Protocol with
  create_capability() -> AbstractCapability | None
- Implement StaticToolsetFactory: wraps pre-configured Tool list,
  produces FunctionToolset/ApprovalRequiredToolset/CombinedToolset
- Implement AdapterToolsetFactory: wraps existing ResourceProvider
  for incremental migration (delegates to provider.as_capability())
- Protocol is structural (duck-typed) matching pdai's own Toolset style

* fix(tools): address Gemini review on PR #98 — extract tool wrapping util, remove dead code

- Extract _wrap_for_pydantic_ai into shared tool_wrapping.py module
- StaticToolsetFactory now calls wrap_tool_for_pydantic_ai directly,
  removing tight coupling to deprecated ResourceProvider (comment #1)
- ResourceProvider._wrap_for_pydantic_ai delegates to shared util
  for backwards compatibility
- Remove redundant 'if not toolsets:' dead code (comment #2)
- Fix return type: AbstractToolset[Any] | None (was AbstractCapability)
- Clean up unused imports (inspect, ModelRetry) from base.py
- Rebase onto latest refactor/thin-wrapper (includes PR #97)

* chore(ci): Phase 7 — add import-linter for boundary enforcement

Add import-linter as dev dependency with 3 forbidden contracts:
1. Server must not import from CLI/commands
2. Config must not import from core
3. ACP package must not import from server

Current status: 3 contracts broken (systemic circular deps).
Violations are pre-existing architectural issues requiring gradual
refactoring across hundreds of files. Contracts are documented as
known violations; fixes will be incremental.

* fix(ci): address Gemini review on PR #99 — add ignore_imports + allow_indirect_imports

- Add ignore_imports for all 3 forbidden contracts listing pre-existing
  direct violations (8 server→cli/commands, 72 config→core, 0 acp→server)
- Set allow_indirect_imports = true for all contracts to keep CI green
  while preventing NEW direct boundary violations
- Comments document the migration path (remove entries as imports are fixed)
- Update openspec tasks.md: mark 7.1-7.3 done, update 7.3/7.10 descriptions
- Mark 5.1 and 5.5 done (PR #98 merged ToolsetFactory protocol + adapters)

* feat(capabilities): Phase 6 — 6 pdai Capability implementations

- LoopDetectionCapability: prevents infinite delegation loops via
  wrap_node_run depth tracking, raises LoopDetectionError at max_depth
- TokenBudgetCapability: enforces token budget per run via
  wrap_model_request, raises TokenBudgetExceededError
- ToolOutputBudgetCapability: truncates tool output via
  wrap_tool_execute when exceeding max_output_chars
- DynamicContextCapability: compacts conversation history via
  before_model_request when approaching context limit
- SkillActivationCapability: dynamic per-turn skill injection via
  before_model_request, supersedes SkillBridgeCapability
- MemoryCapability: persistent key-value memory across turns via
  after_node_run (persist) + before_model_request (inject)

All capabilities implement AbstractCapability with for_run() returning
fresh per-run copies. Wiring into get_agentlet() is blocked on Phase 2
(Run Stream Unification) — hooks only fire via RunExecutor.next(node).

19 unit tests pass, ruff clean.

* chore: move mypy and pytest from pre-commit to pre-push stage

Move heavy hooks (mypy full scan ~16s, pytest orchestrator suite ~30s)
from pre-commit to pre-push to keep commits fast (~2-5s). Pre-push runs
mypy + unit-marked tests as a safety net; full suite still runs in CI.

* chore: ignore .worktrees directory

* fix(capabilities): address Gemini review on PR #100

Critical fixes:
- LoopDetectionCapability: use contextvars.ContextVar for depth tracking
  (instance _depth was reset to 0 after each node_run, and for_run() copies
  lost depth across agent delegation boundaries)
- MemoryCapability: inject into SystemPromptPart.content (ModelMessage has
  no system_prompt attribute; getattr always returned None)
- SkillActivationCapability: same SystemPromptPart fix

High fix:
- MemoryCapability for_run(): share _store dict reference instead of shallow
  copy (memories were discarded after each run)

Medium fixes:
- DynamicContextCapability: log warning when compaction triggered but no fn
- ToolOutputBudgetCapability: reuse self._truncate() in case str() block
- Add unit tests for DynamicContext, SkillActivation, Memory, and
  _inject_into_system_prompt helper

CI fixes:
- Fix mypy: move imports from pydantic_ai.agent/result to
  pydantic_ai.capabilities (AgentNode, NodeResult, WrapNodeRunHandler, etc.)
- Fix mypy: for_run() must be async (parent declares async)
- Fix mypy: handler signatures (positional args, not keyword)
- Fix ruff format

Verified: 54 tests pass, ruff clean, mypy clean (7 files, 0 issues)

* feat(scripts): Phase 8 — automated rename script agentpool → agentwolf

Python script that performs one-shot mechanical rename:
- Renames 10 src/ directories (agentpool → agentwolf, agentpool_* → agentwolf_*)
- Replaces all Python imports (1215 files in dry run)
- Updates pyproject.toml package names and entry points
- Updates YAML configs, Markdown docs, JSON schemas
- Excludes openspec/changes/ and .omo/ (historical artifacts)

Usage: python scripts/rename_to_agentwolf.py [--dry-run]

Pre-requisite: all Phase 1-7 PRs merged into refactor/thin-wrapper.
The rename must be executed as a single atomic commit after merge.

* fix(scripts): address Gemini review on PR #101

- EXCLUDE_DIRS: use Path.is_relative_to() instead of string 'in parts'
  (multi-segment paths like 'openspec/changes' were never matched)
- rename_directories: refuse shutil.move if destination already exists
  (prevents nesting src/agentwolf/agentpool on repeated runs)
- Remove redundant update_pyproject() — pyproject.toml already handled
  by replace_references() via *.toml pattern
- check_entry_points: warn on missing files instead of silent skip
- Pre-compile EXCLUDE_DIRS as list[Path] (avoid per-iteration allocation)
- Update docstring steps to match new structure

* docs(openspec): sync tasks.md + create 5 follow-up changes for incomplete phases

tasks.md fixes:
- Mark Phase 3 tasks 3.1-3.14 as [x] (PR #96 merged, was never updated)
- Add task 8.0 for rename script (PR #101 merged)

5 follow-up openspec changes created for remaining work:
- followup-team-removal: Phase 4 — remove Team/TeamRun, migrate 50 callers
- followup-toolset-factory-migration: Phase 5 — create 3 factories, migrate 70 callers, remove ResourceProvider
- followup-capability-wiring: Phase 6 — YAML config support, Agent class wiring, hook audit
- followup-server-boundary-fixes: Phase 7 — fix 80 import-linter violations, add CI
- followup-agentwolf-rename-execution: Phase 8 — execute rename script, verify

Status: 64/123 tasks done (52%). Phases 1-3 100%, Phase 4 50%, Phase 5 12%, Phase 6 76%, Phase 7 27%, Phase 8 4%.

* fix(openspec): add scenarios to all 5 follow-up change specs

All 5 follow-up openspec changes failed validation because spec.md
requirements lacked required #### Scenario: blocks. Added scenarios
to all 25 requirements across 5 spec files.

Also fixed: SkillBridgeCapability requirement now contains SHALL
(was flagged for missing SHALL/MUST keyword).

All 5 changes now pass 'openspec validate'.

* refactor(openspec): merge 5 follow-up changes into single followup-thin-wrapper-refactor

Consolidates 5 separate follow-up openspec changes into one unified
change with 5 spec modules and 63 tasks:

- specs/team-removal: Phase 4 (11 tasks)
- specs/toolset-migration: Phase 5 (15 tasks)
- specs/capability-wiring: Phase 6 (15 tasks)
- specs/boundary-fixes: Phase 7 (11 tasks)
- specs/rename-execution: Phase 8 (11 tasks)

Removed: followup-team-removal, followup-toolset-factory-migration,
followup-capability-wiring, followup-server-boundary-fixes,
followup-agentwolf-rename-execution

Validated: openspec validate followup-thin-wrapper-refactor passes.

* feat: unify tool interception to pydantic-ai capabilities

Replace dual-path tool interception (wrap_tool legacy + capability chain)
with a unified _ToolInterceptCapability that handles confirmation, hooks,
error handling, and injection uniformly across all tool sources (direct,
MCP, ACP).

Changes:
- Add _ToolInterceptCapability in hook_manager.py with get_wrapper_toolset
  (ApprovalRequiredToolset), prepare_tools, wrap_tool_execute,
  before_tool_execute (ModelRetry on deny), after_tool_execute
  (modified_output/additional_context/injection consumption)
- as_capability() returns CombinedCapability[_ToolInterceptCapability, hooks_cap]
  with hooks_cap stripped of tool callbacks (Decision 2)
- Remove if not self.hooks guard in get_agentlet() — always register
- Simplify wrap_tool() to only AgentContext injection + deferred execution
- Remove _execute_with_hooks, handle_confirmation, _handle_confirmation_result
- Remove redundant mode == 'never' auto-approve in approval_bridge.py
- Update 2 tests for new never-mode behavior (bridge routes to provider)
- Update 3 ACP snapshots (ToolResult now converts to ToolReturn)

Spikes verified:
- ModelRetry from before_tool_execute is caught by pydantic-ai (Spike 0.1)
- Nested ApprovalRequiredToolset short-circuits via ctx.tool_call_approved (Spike 0.2)
- Hooks._registry supports stripping individual callbacks (Spike 0.3)

Closes openspec change: unify-tool-interception-to-pydantic-ai-capabilities

* test: add _ToolInterceptCapability unit and integration tests (tasks 5.1-5.13)

- 10 unit tests: get_wrapper_toolset (always/never/per_tool), wrap_tool_execute
  (error handling + pass-through), before_tool_execute (modified_input + deny
  via ModelRetry), after_tool_execute (modified_output, additional_context,
  injection consumption)
- 3 integration tests: hooks fire for MCP tools, confirmation works for MCP
  tools when mode=always, no double-firing when old AgentHooks is active
- All 43/43 tasks complete for unify-tool-interception-to-pydantic-ai-capabilities

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

* fix: address PR #106 review comments — duration tracking in hook_manager

- hook_manager.py: store tool start time in wrap_tool_execute via
  _tool_start_times dict on agent_ctx, compute real duration_ms in
  after_tool_execute instead of hardcoded 0.0
- Replace hasattr with getattr + explicit None check per type-safety rules
- base_team.py review comments (try-except, None guard) are auto-resolved:
  the referenced code (_close_scoped_team_nodes, _resolve_member_prompt)
  does not exist in origin/refactor/thin-wrapper after rebase

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

* refactor: Phase 4 — merge Team/TeamRun into BaseTeam with mode parameter

Remove Team and TeamRun classes, consolidating their execution logic
into a single concrete BaseTeam that dispatches based on mode
('parallel' or 'sequential').

Changes:
- BaseTeam is now concrete with mode parameter (was abstract)
- Team.execute() → BaseTeam._execute_parallel() (Fork+Join graph)
- TeamRun.execute() → BaseTeam._execute_sequential() (chained graph)
- execute_iter(), run(), run_iter(), run_stream() all dispatch by mode
- ExtendedTeamTalk and _TeamRunGraphState moved to graph_team.py
- __or__/__and__ operators return BaseTeam instead of Team/TeamRun
- TeamConfig.get_team() creates BaseTeam instead of Team/TeamRun
- All 17 caller files updated to use BaseTeam
- team.py and teamrun.py deleted (1081 lines removed)
- tasks.md updated with Phase 4 completion status

Verification: 4218 tests pass, mypy clean, ruff clean

* feat: Phase 5 — create 3 ToolsetFactory classes, add deprecation warnings

Add MCPToolsetFactory, LocalSkillToolsetFactory, PoolToolsetFactory
to tools/factory.py, each wrapping their respective ResourceProvider
and implementing the ToolsetFactory protocol.

Add DeprecationWarning to ResourceProvider.__init__,
CodeModeResourceProvider.__init__, and
RemoteCodeModeResourceProvider.__init__ pointing users to
ToolsetFactory-based approach.

Suppress internal DeprecationWarning cascades in ToolManager and
MCPManager since their internal ResourceProvider usage is an
implementation detail of deprecated code.

Verification: 4218 tests pass, mypy clean, ruff clean

* feat: Phase 6 — complete capability wiring, hook audit docs, reconciliation

- Add hook migration audit to hook_manager.py module docstring
  (pre_run, post_run, pre_tool_use, post_tool_use status)
- Add 6 typed capability config models in capabilities.py
  (LoopDetection, TokenBudget, ToolOutputBudget, DynamicContext,
  SkillActivation, Memory) as discriminated union with backward
  compat fallback to GenericCapabilityConfig
- Improve handle_capabilities validator with typed validation
- Add capabilities parameter to Agent.__init__()
- Add reconciliation docstrings to SkillActivationCapability and
  ToolOutputBudgetCapability
- Add capability hooks verification tests for graph run paths
- Update tasks.md with Phase 6 completion status

Verification: 446 agent/capability tests pass, mypy clean, ruff clean

* ci: Phase 7 — add lint-imports to CI pipeline, fix new import violation

* docs: mark Phase 8 as blocked — deferred by user, requires explicit confirmation

* feat(mcp): add _toolset_cache to MCPManager for MCPToolset connection reuse

as_capability() now caches MCPToolset instances by client_id, avoiding
duplicate MCP connections for the same server config. disconnect_all()
closes cached toolsets via __aexit__ (with ValueError guard for
unentered toolsets) before clearing the cache.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

* test(mcp): update tests for _toolset_cache caching behavior

Update 9 tests across 5 files to accept cached MCPToolset instances.
Add regression test verifying cross-task shared toolsets don't raise
CancelScope errors. Add tests for distinct client_ids and cache cleanup.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

* chore(openspec): archive migrate-to-mcptoolset change

All implementation tasks complete (40/41). Task 8.8 (manual QA) deferred.
Delta spec not synced to main specs — thin-wrapper refactor will redefine
MCP integration specs.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

* fix(test): replace isinstance with Annotated Union-safe helper in capabilities_yaml tests

CapabilityConfig is an Annotated[Union[...]] type which cannot be used
with isinstance(). Replace with a helper that checks against concrete
config types. Adapted from commit 0ea40eb (lost in force-push).

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

* refactor: Phase 4 + Phase 7 (server→cli) — remove TeamConfig.get_team(), move NodeCommand to core, fix 8 server→cli import violations

Phase 4:
- Remove TeamConfig.get_team() from agentpool_config/teams.py (config→core violation)
- Move team creation logic to _build_team_from_config() in session_pool.py (core layer)
- Update resource_providers/pool.py to use _build_team_from_config()
- Fix stale docstrings referencing removed Team/TeamRun classes
- Remove 3 ignore_imports entries for agentpool_config.teams

Phase 7 (server→cli):
- Move NodeCommand and AgentCommand from agentpool_commands.base to agentpool.commands.base
- agentpool_commands.base now re-exports for backward compatibility
- Update 7 ACP server command files to import from agentpool.commands.base
- Use importlib for runtime agent_cli import in debug_commands.py
- Remove all 8 server→cli ignore_imports entries

* feat: Phase 6 — wire capabilities: YAML config section into Agent class

- Add capabilities field to BaseAgentConfig (list[CapabilityConfig])
- Wire capabilities from config into Agent.from_config() via build_capability()
- Add _build_capability_from_config() helper with late import to avoid
  config→core static dependency
- Supports both built-in capabilities (loop_detection, token_budget, etc.)
  and generic import-path capabilities

* docs: Phase 6 — hook audit document for Capability overlap analysis

Documents the audit of 4 existing hooks (pre_run, post_run, pre_tool_use,
post_tool_use) against pydantic-ai Capability hooks. Results:
- pre_run/post_run: kept (run-level lifecycle, no Capability equivalent)
- pre_tool_use/post_tool_use: migrated to _ToolInterceptCapability (PR #106)

* docs: update tasks.md — mark Phase 4/5/6/7 completed items

Phase 4: 4.11-4.13, 4.16-4.18 completed (Team/TeamRun removed, get_team() removed)
Phase 5: 5.2-5.4, 5.10 completed (3 factories exist, deprecation warnings added)
Phase 6: 6.14-6.16 completed (hook audit, YAML config, Agent wiring)
Phase 7: 7.4-7.7 completed (server→cli violations fixed, 3 config→core fixed)

* docs: update tasks.md — mark Phase 4/6/7 completed items, defer Phase 7 config→core violations to #114

Phase 4 (Team Cleanup):
- 4.10: Translator tested against all teams: YAML configs (28 tests pass)
- 4.14: All TeamRun callers already migrated to BaseTeam (zero direct imports remain)
- 4.15: _TeamGraphState/_TeamRunGraphState are active internal implementation, not legacy

Phase 6 (Capabilities):
- 6.17: tests/agents/ + tests/capabilities/ all pass (446 tests, 0 failures)

Phase 7 (Server Boundaries):
- 7.8/7.10: 71 config→core violations deferred to #114 (import-linter detects
  TYPE_CHECKING and lazy imports; needs architectural decision on config↔runtime
  separation — 42 TYPE_CHECKING, 1 function-level, 5 module-level)
- 7.11: Full test suite passes (CI green)

Followup tasks.md updated to reflect actual completion status.

* fix: resolve merge conflict — apply PR #111 last_active_at update to session_controller.py

PR #111 (4c38b2b) updated SessionController.receive_request() to refresh
last_active_at on every incoming message, preventing premature TTL cleanup.
After Phase 1 core.py split, SessionController moved to session_controller.py.
This commit applies the same fix to the new file location.

* chore: remove 2,318 lines of dead test code

- Delete test_concurrent_safety.py (entire file unconditionally skipped)
- Delete 3 collect_ignore files (phase2_native_queue, steer_followup_edge_cases, steer_followup_integration)
- Delete test_input_provider.py (6 xfail tests for unimplemented RFC-0015)
- Delete test_message_timeout.py (pre-SessionPool code path)
- Delete src/acp_v2/ and tests/servers/acp_server/v2/ ghost directories (.pyc only)
- Remove 8 run/turn separation skip tests across 5 files
- Remove 4 tests referencing removed APIs (pool.get_agents, SubagentTools.task)
- Clean up collect_ignore entries from conftest.py
- Fix unused imports flagged by ruff

---------

Co-authored-by: Test <test@test.com>
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Leoyzen added a commit that referenced this pull request Jul 7, 2026
…requests (#110)

* feat(sessions): extend PendingDeferredCall for elicitation, add ElicitationResumePayload and ElicitationDeferredEvent

- Extend deferred_kind with 'elicitation' variant
- Add optional fields: elicitation_message, elicitation_schema, elicitation_mode, mcp_server_id
- Add ElicitationResumePayload Schema for accept/decline/cancel responses
- Add ElicitationDeferredEvent dataclass to RichAgentStreamEvent union
- Serialization backward-compatible (optional fields default to None)

Refs: #107

* feat(agents): add ElicitationResolutionStrategy abstraction with checkpoint and protocol placeholders

- Define ElicitationResolutionStrategy Protocol with async resolve() method
- Implement CheckpointResolutionStrategy delegating to CheckpointManager.checkpoint()
- Add ProtocolResolutionStrategy placeholder for future MRTR/SEP-2663 support

Refs: #107

* feat(ui): add supports_durable_elicitation property to InputProvider and implementations

- Add supports_durable_elicitation property to InputProvider base class
  (defaults to False; StdlibInputProvider and MockInputProvider inherit
  this default)
- Add dynamic property to ACPInputProvider checking
  self.session.checkpoint_enabled at runtime
- Add dynamic property to OpenCodeInputProvider checking session
  checkpoint config via session controller at runtime
- Add checkpoint_enabled field to ACPSession and SessionState (default
  False) as foundation for runtime checks

Refs: #107

* feat(agents): implement two-level elicitation interception via sentinel and side-channel

Add _pending_elicitation_deferral side-channel to AgentContext for
durable elicitation. When provider.supports_durable_elicitation is True,
handle_elicitation() stores params and returns a sentinel decline result
instead of calling get_elicitation() directly. MCPClient.call_tool()
checks the side-channel after the MCP call returns and raises
CallDeferred with elicitation metadata, enabling checkpoint-based
deferral.

Key design decisions:
- Sentinel return (decline) prevents FastMCP from blocking on the handler
- Side-channel dict on AgentContext avoids exception-based control flow
  inside the elicitation callback (FastMCP catches exceptions)
- except CallDeferred: raise placed BEFORE broad except Exception to
  prevent CallDeferred from being swallowed into RuntimeError
- Exhaustive match on ElicitRequestParams union (FormParams | URLParams)
  with no getattr/hasattr

Refs: #107

* feat(agents): add ElicitationDeferredBridge capability and ElicitationFutureRegistry, wire into capability chain

- Create elicitation_bridge.py with ElicitationDeferredBridge as a
  HandleDeferredToolCalls capability following the deferred_bridge/approval_bridge
  factory function pattern
- Implement ElicitationFutureRegistry: per-session registry of asyncio.Future
  instances with register/resolve/reject_all lifecycle methods
- Bridge handler inspects DeferredToolRequests.calls for entries with
  metadata['deferred_kind'] == 'elicitation', checkpoints via
  CheckpointManager, emits ElicitationDeferredEvent, registers future,
  and returns None (block). Non-matching calls pass through.
- Wire elicitation bridge into get_agentlet() capability chain AFTER
  deferred_bridge and BEFORE approval_bridge

Refs: #107

* feat(events): emit ElicitationDeferredEvent and handle in ACP + OpenCode converters

- Mark ElicitationDeferredEvent as immediate in _is_immediate() in core.py
- ACP event_converter: emit ToolCallStart with elicitation params in _meta
- OpenCode event_processor: create ToolPart with elicitation metadata

Refs: #107

* feat(orchestrator): extend resume_session for elicitation deferral with in-process and crash recovery paths

- Add elicitation_payloads parameter to resume_session() (optional, default None)
- Add _try_in_process_elicitation_resume() helper: resolves futures in
  ElicitationFutureRegistry when agent run is still alive (in-process)
- Extend _resume_native_agent() with crash recovery: pre-populates
  cached_elicitation_responses on AgentRunContext so handle_elicitation()
  returns cached response during MCP tool re-execution
- Add elicitation_registry and cached_elicitation_responses fields to
  AgentRunContext
- Update handle_elicitation() to check cached responses before deferring
- Store ElicitationFutureRegistry on run_ctx in get_agentlet()
- Add __contains__ to ElicitationFutureRegistry for membership checking
- Add SessionClosedError exception
- Call registry.reject_all(SessionClosedError()) on session close
- Validate elicitation payloads cover all elicitation deferred calls
- Exclude elicitation calls from deferred_tool_results validation

Refs: #107

* test(elicitation): add 14 unit tests for durable elicitation bridge

Covers Tasks 9.1-9.7, 9.14-9.16, 9.18-9.19, 9.21-9.22:
- PendingDeferredCall serialization roundtrip (with/without elicitation fields)
- handle_elicitation durable=True (side-channel + decline sentinel)
- handle_elicitation durable=False (direct get_elicitation call)
- MCPClient.call_tool raises CallDeferred when side-channel is set
- MCPClient.call_tool normal path (no deferral)
- Elicitation bridge handles elicitation + passthrough non-elicitation calls
- ElicitationFutureRegistry register/resolve/reject_all lifecycle
- ACPInputProvider.supports_durable_elicitation dynamic property
- OpenCodeInputProvider.supports_durable_elicitation dynamic property
- Side-channel cleanup on error (finally block clears _current_elicitation_handler)
- Bridge positioning: elicitation handled before approval
- Backward compatibility: old-format PendingDeferredCall deserialization
- ElicitationResumePayload decline/cancel/accept actions + cached response
- Strategy classes: CheckpointResolutionStrategy, ProtocolResolutionStrategy
- Elicitation timeout field serialization/deserialization

Refs: #107

* test(elicitation): add 9 integration tests for durable elicitation bridge

Refs: #107

* fix(tests): set _pending_elicitation_deferral=None in mock agent contexts

AsyncMock returns truthy mocks for any attribute, which incorrectly
triggered the CallDeferred side-channel check in MCPClient.call_tool().

Refs: #107

* docs(elicitation): document durable elicitation flow and MRTR integration path

Refs: #107

* fix(tests): update resume session tests for run_stream() migration

Task 7 changed _resume_native_agent() to call agent.run_stream() instead
of agent.run(). Updated test mocks to use async generators instead of
AsyncMock to properly match the async iterator interface.

Refs: #107

* fix(elicitation): design correction, CI fixes, and PR review feedback

Design correction:
- handle_elicitation() now raises CallDeferred directly (not side-channel)
- MCP elicitation_handler catches CallDeferred, converts to side-channel
  (FastMCP workaround isolated to MCP client only)
- Local tools like question_for_user need zero adaptation

PR review fixes:
- #1: Accumulate all pending_calls, single checkpoint after loop
  (prevents overwrite when multiple parallel elicitation calls)
- #2: Bypass SessionBusyError for in-process elicitation resume
  (add allow_active_run parameter to _with_resume_lock)

CI fixes:
- ruff: D202 blank line after docstring, D104 missing package docstring
- ruff format: 3 files reformatted
- snapshot: update ACP event converter snapshot (pre-existing drift)

Refs: #107

* style: fix ruff format in _with_resume_lock

Refs: #107

* fix(tests): update capability chain tests for elicitation bridge

- test_approval_bridge: expect 3 HandleDeferredToolCalls (deferred + elicitation + approval)
- test_capability_chain: patch elicitation bridge, verify deferred < elicitation < approval order

Refs: #107

* fix(acp): add checkpoint_enabled to _ACPSessionProxy

_ACPSessionProxy is used by ACPProtocolHandler when creating
ACPInputProvider for elicitation. Without checkpoint_enabled,
ACPInputProvider.supports_durable_elicitation raises
AttributeError: '_ACPSessionProxy' object has no attribute
'checkpoint_enabled'.

Refs: #107

* feat(elicitation): auto-enable durable elicitation when storage backend configured

SessionState.checkpoint_enabled now defaults to True when SessionController
has a store (session persistence configured). ACP _ACPSessionProxy also
passes checkpoint_enabled based on store availability. This means durable
elicitation is automatically active for ACP and OpenCode servers when
storage is configured, without requiring explicit configuration.

Refs: #107

* fix(agents): add DeferredToolRequests to output_type when capabilities present

Without DeferredToolRequests in the agent's output_type, pydantic-ai
raises 'A deferred tool call was present, but DeferredToolRequests is
not among output types' when handle_elicitation() raises CallDeferred.
This was the root cause of the production error:
  ❌ Error: [engineer] A deferred tool call was present...

Refs: #107

* test(elicitation): add protocol-layer integration tests

Covers: DeferredToolRequests in output_type, checkpoint_enabled auto-set,
_ACPSessionProxy durable support, bridge handler with real DeferredToolRequests,
ACP event converter with real EventBus, handle_elicitation CallDeferred raise.

Refs: #107

* fix(acp): send elicitation/create request to client for durable elicitation

The ACP event converter was emitting a ToolCallStart NOTIFICATION for
ElicitationDeferredEvent, but the ACP client expects an interactive
elicitation/create REQUEST (request-response, not fire-and-forget).

Now when _handle_event receives ElicitationDeferredEvent:
1. Spawns a background task (non-blocking for event consumer loop)
2. Sends elicitation/create request to ACP client via ACPRequests
3. Waits for user response
4. Builds ElicitationResumePayload from response
5. Calls session_pool.resume_session() with the payload

This closes the loop: handle_elicitation() → CallDeferred → bridge
checkpoint → ElicitationDeferredEvent → ACP elicitation/create → user
responds → resume_session → future resolved (in-process) or crash
recovery.

Refs: #107

* fix(orchestrator): always use crash recovery for elicitation resume

The in-process resume path was fundamentally broken: handle_elicitation()
raises CallDeferred, which ends the agent run with DeferredToolRequests
as output. Nobody awaits the ElicitationFutureRegistry futures, so
resolving them does nothing — the agent run doesn't continue and the
stream is dead.

Fix: always fall through to crash recovery path (_resume_native_agent),
which re-executes the agent with cached_elicitation_responses. This
correctly re-runs the tool, handle_elicitation() returns the cached
response, the tool completes, and the agent produces events normally.

The in-process futures are still resolved (for registry cleanup), but
the early return is removed. The has_in_process_elicitation bypass of
SessionBusyError is kept (old run is completed but still registered).

Refs: #107

* fix: allow 'active' session status in _with_resume_lock when allow_active_run=True

The elicitation bridge checkpoint saves checkpoint data but doesn't
update the session store status from 'active' to 'checkpointed'.
When resume_session() is called with allow_active_run=True (in-process
elicitation resume), the status check was still rejecting 'active'
status, causing SessionBusyError.

Now when allow_active_run=True, both 'checkpointed' and 'active'
statuses are allowed in the persisted session data check.

Refs: #107

* refactor(elicitation): local tools await future instead of raising CallDeferred

Major design correction: handle_elicitation() now has three paths:

1. Crash recovery: returns cached response (unchanged)
2. MCP tools (in_mcp_callback=True): raises CallDeferred (unchanged —
   FastMCP callbacks can't await for long periods)
3. Local tools (in_mcp_callback=False): checkpoints, emits event,
   registers future, and **awaits the future** — agent run suspends
   without ending. When user responds, future resolves, tool completes
   naturally, agent run continues. No re-execution.

Changes:
- context.py: Added checkpoint_manager to AgentRunContext, in_mcp_callback
  to AgentContext, rewrote handle_elicitation() with three-path logic
- agent.py: Store checkpoint_mgr on run_ctx.checkpoint_manager
- client.py: Set agent_ctx.in_mcp_callback=True before MCP call, clear
  in finally block
- core.py: resume_session() now returns early when in-process futures
  are resolved (agent run continues naturally, no crash recovery needed)
- Tests: Split test 9.2 into MCP path (raises CallDeferred) and local
  path (awaits future), updated protocol integration test

Refs: #107

* fix(orchestrator): detect in-process elicitation via registry, not session data

SessionData.pending_deferred_calls is not updated when local tools
suspend on await future (only checkpoint storage is updated). The
in-process detection now checks the ElicitationFutureRegistry directly
instead of relying on session data. Also added __len__ to
ElicitationFutureRegistry for the non-empty check.

Fixes: SessionBusyError when resuming after local tool elicitation

Refs: #107

* test(elicitation): red-flag test for StreamCompleteEvent after future resume

Two-level test covering the full in-process elicitation resume lifecycle:
- Level 1: Direct NativeTurn.execute() — core turn generator
- Level 2: RunHandle.start() → EventBus → consumer — integration layer

Both levels currently pass, confirming the core agent run and RunHandle
integration correctly yield StreamCompleteEvent after future resolution.
If the bug reappears, these tests will pinpoint which layer fails.

Refs: #107

* fix(turn): set ChatMessage.usage from last ModelResponse instead of defaulting to zeros

NativeTurn.execute() was constructing ChatMessage without setting the usage
field, causing it to default to RequestUsage() (all zeros). The ACP event
converter reads message.usage (not message.cost_info) for UsageUpdate
notifications, so clients always saw 0 tokens.

Fix: extract RequestUsage from the last ModelResponse in new_messages and
pass it to ChatMessage constructor.

Refs: #107

* fix: address PR review comments (#110)

1. context.py: Add finally block to remove future from registry on
   timeout/cancellation, preventing ValueError on retry.
2. core.py: Guard against concurrent run if in-process elicitation
   resolution fails — raise SessionBusyError if run is still active.
3. handler.py: Re-raise CancelledError instead of swallowing it.
   User cancel comes as response.action='cancel', not CancelledError.
4. core.py: Fix misleading comment — no filtering needed because
   API contract separates elicitation payloads from deferred results.

Refs: #107

* style: fix ruff format in test_protocol_integration.py

* fix(acp): suppress SessionResumeEvent notification to frontend

Replace ACP AgentMessageChunk ("🔄 Session resumed...") with a
backend-only log. The frontend doesn't need this notification —
streaming events from the resuming agent run are sufficient.

Refs: #107

* feat: make elicitation timeout configurable via agent config

Add  field to BaseAgentConfig (defaults to 300s).
Supports string (5m, 300s), int (seconds), timedelta, or null
(infinite wait). The value flows through AgentRunContext to
handle_elicitation(), replacing the hardcoded 300s.

Also add  to ElicitationDeferredEvent so frontends
can display countdown timers.

YAML example:
  agents:
    my_agent:
      elicitation_timeout: 600s  # or null for infinite

Refs: #107

* fix: address remaining PR review issues (#110)

P0: Pass real message_history to checkpoint in handle_elicitation()
- Add current_messages field to AgentRunContext
- Set it in tool_wrapping.py from ctx.messages before each tool call
- Use run_ctx.current_messages instead of [] in checkpoint call
- Fixes crash recovery re-executing all prior tool calls (duplicate
  side effects)

P1a: Checkpoint failure no longer silently swallowed
- Use logger.warning with explicit message about degraded durability
- Don't set run_ctx.checkpointed=True on failure

P1b: Cancel ACP elicitation tasks on session close
- Cancel pending _elicitation_tasks in _after_consumer_loop
- Cancel and clear tasks in close_session

P2: Update session store status to 'checkpointed' after elicitation
bridge checkpoint
- Both handle_elicitation() (local tools) and elicitation_bridge.py
  (MCP tools) now update session store status from 'active' to
  'checkpointed' after saving checkpoint
- Eliminates the 'liminal' status that required allow_active_run
  workaround in _with_resume_lock
- Wrapped in try/except so failures don't break the checkpoint path

Refs: #107

* test: add coverage for PR review fixes (P0, P1a, P2)

- P0: verify handle_elicitation passes current_messages to checkpoint
  (not empty list) — prevents crash recovery re-executing tools
- P1a: verify checkpoint failure doesn't set checkpointed=True — the
  in-process future await still works but crash recovery is degraded
- P2: verify session store status is updated to 'checkpointed' after
  elicitation checkpoint, and skipped if already checkpointed

Refs: #107

* fix: inline parse_time_period in agentpool_config to fix import layer violation

agentpool_config.nodes was importing agentpool.utils.parse_time,
breaking the 'Config must not import from core' contract.

Inlined a simple regex-based parser in the field_validator to
maintain layer separation.

Refs: #107

* fix(agent): prevent duplicate capabilities from from_config() + get_agentlet()

from_config() pre-built capabilities and stored them in _extra_capabilities,
while get_agentlet() also iterated self.config.capabilities and built them
again — producing duplicate FunctionToolset instances with conflicting tool
names (e.g. two 'task' tools from BackgroundTaskCapability).

Fix: skip pre-building in from_config(); let get_agentlet() handle all
capability construction lazily from self.config.capabilities.

* fix: make _ToolInterceptCapability.hook_manager keyword-only

AbstractCapability base class uses KW_ONLY for its fields (id,
description, defer_loading), all with defaults. The subclass field
hook_manager (no default) was not keyword-only, causing a dataclass
TypeError when running against pydantic-ai v1.x where the base class
is @DataClass(init=False).

* fix(acp): cancel subagent runs when parent session is cancelled

cancel_session() only cancelled the parent session's RunHandle, leaving child (subagent) sessions running independently. Added _cancel_subagent_runs() which recursively walks the _parent_of tree and calls cancel_run_for_session() for each child before cancelling the parent.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>

---------

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Leoyzen added a commit that referenced this pull request Jul 7, 2026
…e leaks

- Fix #1 (Critical): resume_session() now removes session_id from
  _connection_sessions before closing old session. Prevents stale
  connection disconnect from closing the newly resumed session.
- Fix #2 (High): as_capability() uses _session_contexts.get() instead of
  get_or_create_session(). Prevents memory leak when context was already
  cleaned up. Removes dead try/except KeyError code.
- Fix #3 (Medium): cleanup_session() uses _session_contexts.get() and
  returns early if None. Avoids creating throwaway SessionConnectionPool.
- TDD: 3 tests in test_review_fixes.py verify all fixes.
Leoyzen added a commit that referenced this pull request Jul 8, 2026
* spec: MCP session lifecycle fix — Phase 1

Add OpenSpec change for fixing stale MCP toolset cache and session-scoped
resource lifecycle bugs (#121). Includes:

- proposal.md: What & why (6 lifecycle fixes, no config changes)
- design.md: 8 design decisions (D1-D8) with Oracle + Momus review
- specs/mcp-session-lifecycle: 7 requirements, 14 scenarios
- specs/session-orchestration: Modified requirements for close path
- specs/unified-session-lifecycle: WebSocket disconnect hook
- tasks.md: 7 task groups, 46 tasks (P1a-P1f + E2E)
- tests/mcp_server/test_stale_mcp_connection.py: 5 reproduction tests

Reviewed by Momus (PASS) and Oracle (PASS) after 2 revision cycles.

Closes #121 (spec phase)

* spec: address Gemini Code Assist review comments

4 accepted fixes from dialectical analysis with Oracle:

1. Task 3.1/3.2/3.3: Change _session_connections to
   dict[str, set[tuple[str, int]]] — store (connection_id,
   session_key) pairs so AcpMcpConnectionManager.cleanup_session()
   can look up SessionStreamPair via session_key

2. Task 5.2 (D6): Two-layer cleanup on resume — call both
   SessionController.close_session() (RunHandle lifecycle) AND
   ACPSession.close() (ACP env/signals/prompts). Neither alone
   is sufficient.

3. Task 6.4 (D7): Same two-layer cleanup for WebSocket disconnect

4. Task 2.8: Use try/finally or fixture teardown for test cleanup

Rejected comments (2):
- hasattr(self.agent, 'mcp'): violates AGENTS.md, mcp always set
- hasattr(agent, 'mcp'): same, agent is not None check exists

Already addressed (2):
- Concurrency re-verify after lock: spec's lock-on-context design
  handles this implicitly
- await on_disconnect: type signature makes it obvious

* feat(mcp): add _SessionContext dataclass and session connection tracking

- Add _SessionContext dataclass to MCPManager with per-session state
  (connection_pool, toolset_cache, snapshot, acp_connection_ids, _cleanup_lock)
- Add _session_contexts dict to MCPManager.__init__
- Add _session_connections reverse index to AcpMcpConnectionManager
- Add register_session_connection() method for tracking session→connection mappings

Implements T1 and T6 of fix-mcp-session-lifecycle plan.

* feat(mcp): add session lifecycle methods and ACP cleanup

- get_or_create_session() and update_session_snapshot() on MCPManager (T2)
- add_acp_transport() on MCPManager for session-scoped ACP tracking (T3)
- register_session() returns tuple[SessionStreamPair, int] (T7, GAP-1)
- has_active_sessions() on AcpMcpConnection (T7)
- cleanup_session() with _cleanup_lock on AcpMcpConnectionManager (T7, GAP-12)
- Updated all callers of register_session() to unpack tuple return

* feat(mcp): cleanup_session on MCPManager and wire register_session_connection

- cleanup_session() with per-session _cleanup_lock on MCPManager (T4)
- _acp_mcp_manager field added for ACP cleanup delegation
- connect_acp_mcp_server() gains session_id parameter (T8, GAP-5)
- Returns tuple[str, int] (connection_id, session_key)
- Call site in session.py passes session_id and calls add_acp_transport
- All test callers updated for new signature

* test(mcp): add session lifecycle and ACP cleanup unit tests (T5+T9)

* refactor(mcp): change as_capability to session_id-based API (T10)

- Change as_capability(snapshot=, session_pool=) to as_capability(session_id=)
- Parameterize _make_capability with toolset_cache dict parameter (GAP-7)
- Split _process_snapshot into _process_global_configs and _process_session_configs
- GAP-11: KeyError fallback for concurrent cleanup_session race
- Backward compat: session_id=None processes self.servers with self._toolset_cache

* refactor(agent): update get_agentlet to use as_capability(session_id) (T12)

- Replace as_capability(snapshot=, session_pool=) with as_capability(session_id=)
- GAP-4: Use run_ctx.session_id from AgentRunContext instead of self._session_id
- Remove if/else branching on _mcp_snapshot — as_capability handles internally
- Keep _mcp_snapshot and _session_connection_pool field declarations for compat

* test(mcp): update caching+provider tests for session_id API (T13)

- Update 6 tests in test_mcpmanager_caching.py for new as_capability(session_id) API
- Update 15 failing tests in test_mcp_provider_lifecycle.py to use session context
- Fix static source assertion in test_no_dedup_hack_in_get_agentlet
- All 48 tests pass

* test(mcp): flip stale connection tests to verify fix (T14)

- Rename test_session_resume_returns_stale_toolset → _returns_fresh_toolset
- Rename test_multiple_acp_servers_all_go_stale → _get_fresh_toolsets
- Rename test_disconnect_all_clears_cache → test_cleanup_session_clears_per_session_cache
- All 5 tests now verify the fix instead of documenting the bug
- All tests pass with new session_id API

* feat(session): wire cleanup_session into ACPSession.close and SessionController (T15)

* feat(agent): wire get_or_create_session in SessionController agent creation (T16)

* test(mcp): integration tests for session close lifecycle (T17+T18+T19)

* fix(acp): resume_session close-then-recreate instead of early-return (T20)

* test(acp): resume_session lifecycle tests - close, reconnect, active run (T21+T22+T23)

* feat(acp): add on_disconnect callback to websocket handler (T24)

- Add on_disconnect: Callable[[AgentSideConnection], Awaitable[None]] | None parameter
- Generate UUID4 connection_id on AgentSideConnection at accept time (GAP-3)
- Call on_disconnect in ConnectionClosed handler before conn.close()
- Backward compatible: on_disconnect defaults to None

* feat(acp): implement close_all_sessions_for_connection (T25)

- Add _connection_sessions reverse index on ACPSessionManager
- Add connection_id parameter to create_session() and resume_session()
- Implement close_all_sessions_for_connection() for WebSocket disconnect cleanup
- Idempotent: pops connection_id, iterates sessions, closes via SessionController + ACPSession.close()

* feat(acp): wire on_disconnect to close_all_sessions_for_connection (T26)

- Add on_disconnect parameter to serve(), _serve_websocket(), _serve_streamable_http()
- Wire on_disconnect callback in ACPServer._start_async() closure
- Add session_manager field to AgentPoolACPAgent for shared session tracking
- Create shared ACPSessionManager in ACPServer for cross-connection session tracking
- Add disconnect detection in _serve_streamable_http via recv_task completion
- Fix test_resume_session_is_idempotent -> test_resume_session_closes_old_and_recreates
  (T20 changed resume_session from idempotent to close-then-recreate)

* test(acp): websocket disconnect closes sessions and preserves others (T27+T28)

- T27: test_websocket_disconnect_closes_all_sessions — 2 sessions same conn, disconnect, both closed
- T27: test_websocket_disconnect_preserves_other_connections — 2 conns, disconnect one, other survives
- T28: test_websocket_disconnect_during_run — active run cancelled with 2s timeout on disconnect

* fix(acp): resolve mypy union-attr errors with cast (T32) + add e2e session lifecycle test (T33)

- T32: Use cast() to type session_manager field as ACPSessionManager (not | None) for mypy
- T33: test_e2e_session_lifecycle — full lifecycle: connect→session→MCP→disconnect→reconnect→resume→verify fresh

* fix: resolve CI ruff format and lint errors

- ruff format: reformat 5 files (manager.py, session_controller.py, session.py, test_session_lifecycle.py, test_stale_mcp_connection.py)
- ruff check: shorten docstring in test_acp_session_resume.py (E501)

* fix(mcp): address review — _connection_sessions cleanup, get_or_create leaks

- Fix #1 (Critical): resume_session() now removes session_id from
  _connection_sessions before closing old session. Prevents stale
  connection disconnect from closing the newly resumed session.
- Fix #2 (High): as_capability() uses _session_contexts.get() instead of
  get_or_create_session(). Prevents memory leak when context was already
  cleaned up. Removes dead try/except KeyError code.
- Fix #3 (Medium): cleanup_session() uses _session_contexts.get() and
  returns early if None. Avoids creating throwaway SessionConnectionPool.
- TDD: 3 tests in test_review_fixes.py verify all fixes.

* fix(mcp): wire _acp_mcp_manager, add identity check, consolidate as_capability

Review round 2 fixes:

Fix #4 (Critical): Wire _acp_mcp_manager in ACPSession.__post_init__
- MCPManager._acp_mcp_manager was initialized to None and never set
- cleanup_session() could never delegate to AcpMcpConnectionManager
- Per-session ACP stream pairs and reverse-index entries leaked
- Fix: wire agent.mcp._acp_mcp_manager = acp_agent._mcp_manager in __post_init__

Fix #5 (Medium): Identity check after acquiring cleanup lock
- Concurrent cleanup_session() callers could do redundant work
- All ops were idempotent but wasteful (clearing empty dicts, etc.)
- Fix: check if self._session_contexts.get(session_id) is not ctx after lock

Fix #6 (Medium): Consolidate duplicated fallback in as_capability()
- Three identical 'for server in self.servers:' loops consolidated to one
- Pure readability refactor, zero behavior change

TDD: 3 new tests (2 RED before fix, 3 GREEN after)
- test_cleanup_session_delegates_to_acp_mcp_manager (unit)
- test_acp_session_wires_acp_mcp_manager (integration)
- test_cleanup_session_identity_check_prevents_redundant_work (unit)

213 tests pass, ruff clean.

* test(mcp): add 20 integration tests for session wiring lifecycle

Categories A-D from Oracle integration test plan:
- A (4): Cross-component wiring — cleanup delegation, __post_init__ wiring,
  full close chain, close_all_sessions_for_connection
- B (7): Lifecycle edge cases — full create/cleanup, close/recreate,
  shared connection isolation, WebSocket disconnect, resume, concurrent cleanup
- C (4): State consistency — registry consistency after cleanup/close/resume,
  stream pair unregistration
- D (5): Error paths — ACP manager raises, session close raises,
  MCP cleanup raises, resume old close raises, pool cleanup raises

These tests would have caught the _acp_mcp_manager wiring bug (round 2
review comment #1) that unit tests missed due to component isolation.

* fix(acp): wire connection_id through create_session/resume_session call sites

- Declare connection_id: str | None on AgentSideConnection (replaces monkey-patch)
- Remove # type: ignore[attr-defined] from transports.py connection_id assignments
- Add _get_connection_id() helper on AgentPoolACPAgent using isinstance check
- Wire connection_id= into all 5 create_session/resume_session call sites:
  new_session, load_session, fork_session, resume_session, handler.py
- Fix misleading GAP-11 comment: dict.get() returns None, never raises KeyError

Without this fix, _connection_sessions dict was never populated, making
close_all_sessions_for_connection() always return immediately — the entire
WebSocket disconnect cleanup feature was dead code.

* test(mcp): add 13 E2E integration tests for full MCP session lifecycle

Covers all 13 gap areas identified by Oracle analysis:
- G1: Full create_session → get_or_create_session_agent → MCPManager chain
- G2: as_capability with non-empty ACP snapshot → real MCPToolset
- G3: initialize_mcp_servers → connect_acp_mcp_server → AcpMcpTransport
- G4: Full tool execution through as_capability → MCPToolset → AcpMcpTransport
- G5: SessionController.close_session with real agent + real MCP resources
- G6: resume_session with real ACPSession (not patched)
- G7: Full on_disconnect → close_all_sessions_for_connection chain
- G8: connection_id propagation: create_session populates _connection_sessions
- G9: as_capability during concurrent cleanup (GAP-11 race)
- G10: ACP transport failure during tool execution + cleanup
- G11: Multiple sessions on same connection with real ACPSessions
- G12: Child session inherits parent's ACP transports
- G13: Pool shutdown cleans all session MCP resources

* fix: resolve CI mypy and unit test failures

- server.py: Remove unused type: ignore, use None guard for connection_id
- test_acp_session_resume.py: Add connection_id to expected resume_session call args

* chore(openspec): archive fix-mcp-session-lifecycle and sync specs

- Mark all 46 tasks as complete in tasks.md
- Sync 3 delta specs to main specs:
  - mcp-session-lifecycle (new)
  - session-orchestration (updated)
  - unified-session-lifecycle (updated)
- Archive to openspec/changes/archive/2026-07-07-fix-mcp-session-lifecycle/

* fix: parent session memory leak + on_disconnect in finally (review r3)

- session_controller.py: Replace get_or_create_session() with
  _session_contexts.get() when reading parent snapshot/pool. Prevents
  phantom _SessionContext creation when parent was already cleaned up.
- transports.py: Move on_disconnect callback from except ConnectionClosed
  to finally block. Ensures callback fires on any exception path.
- 3 TDD tests: leak detection, regression guard, disconnect coverage.

* fix(mcp): wire child session ACP manager, add transport callback, fix toolset __aexit__

Three fixes for child session ACP transport registration gaps:

1. Wire _acp_mcp_manager on child agent from parent (session_controller.py)
   - Child sessions created via get_or_create_session_agent() don't go
     through ACPSession.__post_init__, so _acp_mcp_manager stayed None.
     Now copied from parent after copy_pre_created_transports().

2. Add on_session_registered callback to AcpMcpTransport (acp_mcp_transport.py)
   - Optional callback invoked after register_session() with (connection_id,
     session_key). Enables callers to register ACP connections for cleanup
     tracking via register_session_connection().

3. Fix toolset_cache.clear() to call __aexit__ first (manager.py)
   - cleanup_session() called .clear() without closing MCPToolset instances,
     leaking stream pairs and forwarder tasks. Now mirrors disconnect_all()
     pattern: iterate values, call __aexit__(None, None, None) with
     contextlib.suppress(ValueError), then clear.

TDD: 3 tests in test_child_session_acp_fix.py (all GREEN).
252 MCP+ACP tests pass, 0 regressions, ruff clean.
Leoyzen added a commit that referenced this pull request Jul 20, 2026
…line

Bug #1: send_message to non-existent member created phantom entry in
team state because bounds check (turn_count increment) ran before the
get_member_session_id check. Fix: move member existence check before
bounds check so non-existent members are rejected without touching state.

Bug #2: shutdown_request closed the session but didn't update team state,
so team_status still showed the member with a stale session_id. Fix:
clear session_id and set status='shutdown' in team state after closing.

New tests:
- test_send_message_to_nonexistent_member_no_phantom
- test_shutdown_request_marks_member_offline
156 tests pass.
Million-mo pushed a commit that referenced this pull request Jul 28, 2026
* fix: add span instrumentation for critical-path methods (fix-span-instrumentation)

Fixes orphan traces in subagent sessions by adding logfire span
instrumentation to RunLoop, Turn, delegation, capabilities, lifecycle,
graph, and ACP layers.

P0 (span breakage fix):
- SubagentCapability.spawn_subagent(): delegation.subagent span
- RunLoopDelegationService.spawn_subagent(): fix double-iteration bug + span
- RunHandle.start(): safe_span for async generator
- RunHandle._execute_turn(): safe_span for async generator
- NativeTurn.execute(): safe_span for async generator
- ACPTurn.execute(): safe_span for async generator
- Create safe_span() helper to suppress OTel context detach ValueError

P1 (coverage expansion):
- SessionController: @logfire.instrument on receive_request, _start_run_handle, _consume_run
- SessionPool: @logfire.instrument on steer, followup
- RunHandle: @logfire.instrument on steer, followup
- BaseAgent.run_stream(): safe_span
- BaseTeam: @logfire.instrument on _execute_parallel, _execute_sequential
- subagent_tools: safe_span for background task
- DurableJournal/SnapshotStore: @logfire.instrument
- Graph adapter + signal adapter: @logfire.instrument
- ACP cross-process: TraceContextTextMapPropagator inject/extract

Tests:
- 4 span hierarchy tests (delegation, team parallel/sequential, bg task)
- 3 ACP traceparent tests (injection, roundtrip, no-span skip)
- Fixed deprecation test for EventBus subscription pattern

OpenSpec change: openspec/changes/fix-span-instrumentation/

* fix: add entry-point spans to fix orphan traces in ACP path

- SessionPool.send_message(): @logfire.instrument('session.send_message')
- SessionController._route_message(): @logfire.instrument('session.route_message')
- ACP acp_agent.prompt(): replace raw OTel span with logfire.span() for
  consistent context propagation, use attach/detach for W3C trace context

These fill the gap between acp.agent.handle_prompt and _start_run_handle,
ensuring all child spans nest under the root entry-point span.

* fix: remove f-string templates from span names and fix missing span in background task

- Remove {session_id} from all @logfire.instrument format strings in
  session_controller.py and session_pool.py. Logfire span names are
  always the template string (not rendered), so {session_id} showed
  literally in SigNoz. Session_id is still captured as a span attribute
  via logfire's extract_args default.

- Replace @logfire.instrument decorator on _consume_run() with manual
  with logfire.span(...) inside the method body. The decorator may not
  properly establish parent-child span relationship when the coroutine
  runs in a copied contextvars Context (asyncio.create_task). Manual
  span creation ensures the span is created in the correct context.

- Update test mock assertions to match new parameter passing for
  send_message and receive_request calls.

* fix: call _end() separately in safe_span to prevent Missing Span

LogfireSpan.__exit__ is decorated with @handle_internal_errors which
catches ValueError from _detach() and suppresses it — but this also
prevents _end() from being called, since the exception fires in
_detach() before _end() is reached. An unended span is never exported
by the OTel exporter, causing 'Missing Span' in SigNoz.

This affected ALL safe_span callers in async generators:
- session.consume_run
- orchestration.run_handle.start
- orchestration.run_handle.execute_turn
- turn.native / turn.acp
- agent.run_stream
- delegation.subagent (runloop_delegation)

Fix: call _detach() and _end() separately in safe_span's finally block,
each wrapped in suppress(Exception). This ensures _end() always runs
even when _detach() fails due to contextvars Context mismatch.

* fix: use safe_span for session.consume_run and delegation.subagent

session.consume_run in session_controller.py and delegation.subagent in
subagent_capability.py were using raw 'with logfire.span(...)' instead
of safe_span. Logfire's @handle_internal_errors on LogfireSpan.__exit__
catches ValueError from _detach() and suppresses it, but this also
prevents _end() from being called — the span is never ended and never
exported, causing 'Missing Span' in SigNoz.

safe_span calls _detach() and _end() separately, ensuring the span is
always ended even if context detach fails.

This fixes the remaining Missing Span entries that were not addressed
by the previous safe_span fix (commit b2b1cfd16).

* fix: wrap nested async generators with aclosing() to prevent span leaks

When RunHandle.start() is closed via aclose(), the nested async
generators _execute_turn() and turn.execute() were NOT automatically
closed. Their safe_span finally blocks never ran, so spans were never
ended, never exported, causing Missing Span in SigNoz.

Fix: wrap each async-for-over-subgenerator with contextlib.aclosing(),
which calls aclose() on __aexit__, cascading GeneratorExit through all
nested safe_span context managers.

Changes:
- run.py start(): aclosing(self._execute_turn(...))
- run.py _execute_turn(): aclosing(turn.execute())
- test_span_hierarchy.py: add red test (xfail) + green test

* chore(deps): add filelock>=3.13 for team state file locking

* feat(config): add TeamModeConfig model for dynamic team mode

* feat(config): add team_mode field to manifest and agent config

* feat(capabilities): add team_mode_config to AgentContext

* feat(capabilities): add FileTeamState for file-based team persistence

* feat(capabilities): add TeamCommCapability skeleton and factory registration

* feat(capabilities): implement universal team communication tools

Add 8 universal team tools to TeamCommCapability:
- send_message: deliver messages to teammate inboxes via SessionPool
- task_create: create tasks on shared task board with dependency tracking
- task_list: list all tasks as JSON
- task_update: update task status/owner
- read_blackboard: read key with version metadata
- write_blackboard: write with optimistic locking
- list_blackboard: list all blackboard keys
- team_status: formatted team status with member info

All tools extract AgentContext from RunContext.deps, use FileTeamState
for persistence, and handle error cases (no team_id, no session_pool,
member not found, etc.). 19 new unit tests, 35 total pass.

* feat(capabilities): implement lead-only team management tools

* feat(capabilities): enforce team mode bounds at runtime

* feat(capabilities): add TTL cleanup for expired team state

* feat(capabilities): implement auto_init for session-startup team creation

* test(integration): add team mode integration tests

* docs: add team mode documentation and examples

* fix: resolve ruff format and mypy errors in team mode files

* fix: address PR review — hierarchical key mkdir, orphaned session cleanup

* fix: resolve CI lint and format errors in test files

* refactor: remove auto_init, use delegation.create_child_session in team_create

- Remove _maybe_auto_init() and all 12 call sites
- team_create now uses delegation.create_child_session() which emits SpawnSessionStart
- team_create supports config default members from auto_init config
- Update tests: remove auto_init tests, add config default member tests

* refactor: rename auto_init to defaults, clean up docs and examples

- Rename AutoInitConfig → TeamDefaultsConfig, auto_init field → defaults
- Update all source, tests, examples, RFC, AGENTS.md
- Examples now explain that team tools are auto-wired by factory
- Rename test_team_auto_init.py → test_team_defaults.py

* fix: factory per-session TeamCommCapability not added when compile step skipped

- Factory create_session_agent() only replaced existing TeamCommCapability
  instances, but when compile() wasn't called, _extra_capabilities was
  empty and the per-session instance was silently dropped
- Fix: append team_cap if no existing TeamCommCapability found
- Fix: validator now checks member.agent (agent name) not member.name
  (display name) against member_eligible
- Update test data to match corrected validator behavior
- 109 tests pass

* fix: ctx parameter typed as Any instead of RunContext in team tools

PydanticAI uses type annotations to determine which parameters are
auto-injected vs exposed to the LLM. ctx: Any was treated as a regular
parameter, causing the LLM to pass the string 'ctx' instead of the
framework auto-injecting RunContext.

Changed all 12 tool functions + _resolve_agent_context helper from
ctx: Any to ctx: RunContext[Any]. Import RunContext from pydantic_ai.tools
at runtime (not TYPE_CHECKING) so PydanticAI can resolve the annotation.

109 tests pass, ruff + mypy clean.

* fix: resolve AgentContext from PydanticAI runtime context deps.data

Team tools were failing with 'AgentContext object has no attribute session'
because _resolve_agent_context was casting ctx.deps directly to
capabilities.agent_context.AgentContext, but ctx.deps is actually
agents.context.AgentContext (PydanticAI runtime context). Our
AgentContext is stored at ctx.deps.data, set by NativeTurn.

* fix: set team_role metadata in factory create_session_agent

Protocol servers don't set team_role in session metadata, so team tools
couldn't determine if the agent was a lead or member. Factory now sets
team_role='lead' for lead_eligible agents and 'member' for others,
plus team_member_name, before creating TeamCommCapability.

* fix: write team_id back to session.metadata after team_create

team_create returned success but did not write team_id to
session.metadata, causing all subsequent tool calls to fail
with 'Not in a team session'. Now writes team_id and team_name
to session.metadata before returning success.

* feat: inject agent descriptions into team mode instructions

Add agent_descriptions parameter to TeamCommCapability so the LLM can
see what each eligible agent does. Factory extracts descriptions from
manifest (description field, or first line of system_prompt as fallback).

This prevents the LLM from guessing wrong agent names in team_create.

* fix: auto-close member sessions when lead run terminates

* test: add L2 FunctionModel flow tests for team lifecycle

* feat: role-aware tool filtering via prepare_tools + role-specific instructions

- Override prepare_tools() in TeamCommCapability to filter lead-only
  tools (team_create, team_delete, delete_blackboard, shutdown_request)
  for non-lead members — the LLM never sees these tools.
- Strip broadcast (to="*") from send_message schema for non-lead
  members: update description + add pattern constraint rejecting "*".
- Add role-specific capabilities section to get_instructions():
  lead sees broadcast + lead-only tools; member sees individual
  messaging only with explicit 'broadcast not available' guidance.
- Add 7 unit tests covering prepare_tools filtering, schema
  modification, and role-aware instructions.

* fix: add create_child_session to DelegationService test stubs

_StubDelegationService and FakeDelegationService were missing the
create_child_session method added to the DelegationService Protocol,
causing isinstance() checks to fail.

* feat: log team state directory path on team_create

* fix: adapt team_session_leak test to merged session lifecycle changes

After merging main, TestModel runs complete instantly and RunHandle is
removed from _runs. Restructured test to manually inject a RunHandle
instead of relying on send_message timing. Manually set complete_event
after close() since the start() generator's finally block never runs
for a manually created RunHandle.

* test: redesign team mode test strategy with L1-L4 layered architecture

Infrastructure fixes:
- Fix double pytest_addoption in conftest.py (--run-real-models was shadowed)
- Migrate test_team_live.py from custom run_real_models to @pytest.mark.real_model + @pytest.mark.e2e
- Update real_model auto-skip to check MODEL_GATEWAY_URL in addition to OPENAI_API_KEY

New test infrastructure:
- tests/fixtures/team_mode_pool.py: shared team_mode_pool + team_mode_pool_with_defaults fixtures
- tests/team_mode/conftest.py: message inspection helpers (_tool_returns_by_name, _tool_call_names),
  build_agent_context(), make_mock_run_context(), make_lifecycle_model() FunctionModel factory
- tests/vcr/conftest.py: vcr_team_pool fixture for L3 VCR tests

Test migration (git mv, preserves history):
- L1: test_team_comm_capability.py → tests/team_mode/test_unit_capability.py (66 tests)
- L1: test_file_team_state.py → tests/team_mode/test_unit_file_state.py (29 tests)
- L1: test_team_mode_config.py → tests/team_mode/test_unit_config.py (10 tests)
- L2: test_team_flow.py → tests/team_mode/test_flow_lifecycle.py (5 tests)
- L2: test_team_session_leak.py → tests/team_mode/test_integration_session.py (1 test)
- L2: test_team_mode.py → tests/team_mode/test_integration_mode.py (10 tests)
- L2: test_team_defaults.py → tests/team_mode/test_integration_defaults.py (3 tests)
- L4: test_team_live.py → tests/team_mode/test_live.py (4 tests)

New L2 integration tests (FunctionModel patterns from pydantic-ai-harness):
- test_flow_role_aware.py (5 tests): prepare_tools role-aware filtering
- test_flow_error_containment.py (3 tests): soft/hard error isolation
- test_flow_budget.py (3 tests): max_members, max_member_turns, wall_clock enforcement
- test_integration_parallel.py (2 tests): multiple ToolCallParts in one ModelResponse

L3 VCR framework (tests/vcr/test_team_mode.py):
- Case dataclass pattern with 3 parameterized scenarios
- cassette_exists() skip guard — all tests skip until cassettes recorded

L4 E2E smoke (tests/e2e/test_team_mode_smoke.py):
- serve-opencode subprocess + TestModel + team_mode
- SSE event collection and assertion

Total: 137 tests pass, 3 VCR skip (no cassettes), 3 live deselect (no API key)

* test: migrate MagicMock tests to real pool + fix VCR cassette replay

- Migrate 3 MagicMock-heavy tests in test_integration_mode.py to use
  real AgentPool + SessionPool via team_mode_pool fixture
- Fix VCR test matching: override vcr_config with lenient match_on
  (method+path only) to handle model/endpoint differences between
  recording and replay
- Update CASES expected_tool_calls to match actual model behavior
  recorded in cassettes (lead-only tool calls, not member calls)
- Override fail_partially_used_vcr_cassettes for team-mode tests
  (member session interactions naturally remain unplayed)
- Add recorded VCR cassettes for all 3 test cases
- 140 tests pass, 3 VCR tests pass, 3 live tests deselected

* fix: rewrite E2E smoke test to match existing SSE patterns

Root cause: POST /session/{id}/message returns 500 due to pre-existing
OpenTelemetry _IncludedRouter.path bug (issues #185, #190). This affects
ALL serve-opencode E2E tests, not just team mode.

Previous test used /prompt_async + background asyncio.create_task for
SSE collection — wrong pattern. Rewrote to match existing passing tests:
- test_team_mode_server_starts: SSE /event stream → server.connected
  handshake (proves server starts with team_mode)
- test_team_mode_session_creation: POST /session (proves config loaded)
- test_team_mode_prompt_delivery: @xfail @known_bug for OTel 500 bug

Result: 2 passed, 1 xfailed (expected failure for known bug)

* fix: VCR tests fail in CI — OpenAI client needs API key at init

Root cause: OpenAI Python client checks OPENAI_API_KEY during
initialization (before VCR can intercept HTTP requests). In CI
without the key, client init fails → model can't be created →
Invoked: [] (no tool calls).

Fix:
- vcr_team_pool fixture: set dummy OPENAI_API_KEY via monkeypatch
  (VCR intercepts all HTTP requests, key never actually used)
- _skip_if_no_cassette: handle record_mode=None (pytest-recording
  registers default=None, not 'none'), check both cassette paths
  (tests/vcr/cassettes/ and tests/cassettes/vcr/)

* fix: adapt VCR tests to PR #202 infrastructure + filter litellm requests

- Move cassettes to tests/cassettes/vcr/test_team_mode/ (PR #202 convention)
- Filter out litellm GET requests from cassettes (raw.githubusercontent.com)
- Trim unused member-session interactions from cassettes
- Remove vcr_config override (use main's match_on: ['method'])
- Remove fail_partially_used_vcr_cassettes override (use main's strict check)
- Remove allow_model_requests param (main's autouse fixture handles it)
- Fix _skip_if_no_cassette: support VCR_RECORDING env var + correct path
- ruff format e2e test

* fix: handle parametrized test names in check_cassettes.py

Cassette filenames for parametrized tests include the parametrize ID
in brackets (e.g. test_team_mode_via_vcr[create_team].yaml). The
check script was looking for a function named exactly that, which
doesn't exist — the actual function is test_team_mode_via_vcr with
@pytest.mark.parametrize.

Fix: strip [param_id] suffix before checking function existence.

* fix: register lead as team member + idle-based cleanup

- Register lead in team state so members can send_message to lead
- team_delete skips lead's own session when closing members
- broadcast skips lead's own session (no self-delivery)
- Replace complete_event.wait(300s) with last_active_at polling:
  - Polls every 30s (configurable via _poll_interval)
  - Closes members after 300s idle (configurable via _idle_timeout)
  - Correctly handles OpenCode sessions that stay alive between turns
- Update tests for lead registration and polling-based cleanup

* feat: add team_add_member and team_remove_member tools

- team_add_member: dynamically add members to existing team
  - lifecycle: 'ephemeral' (auto-close after run) or 'persistent' (default)
  - notify: str | None — broadcast message to existing members if provided
  - writes blackboard entry for team history
  - respects bounds.max_members and member_eligible checks
- team_remove_member: remove member from team
  - closes session and removes from team state
  - blackboard contributions persist (other members can still read)
- ephemeral cleanup: polls member run state every 5s, auto-closes
  when run completes, then removes from team state
- 14 team tools total (12 existing + 2 new)
- 9 new unit tests, all 146 tests pass

* feat: show team_id and state dir in team_status output

* fix(opencode): handle duplicate message ID in append_message_to_session (#229)

In the sync path (POST /message), the REST handler pre-stores the
assistant message before the event bridge tries to store it with the
same canonical ID (via _pending_message_ids). This caused ValueError:
Duplicate message ID from the storage provider, repeated for every
PartDeltaEvent (850+ errors per request in production logs).

Fix: append_message_to_session now catches ValueError for duplicate
message IDs and treats the write as idempotent (skip + debug log).
The in-memory dict also skips duplicate appends.

Test infrastructure fixes (conftest.py):
- _mock_append_message: check for duplicate message IDs like real
  MemoryProvider (was: unconditional append)
- _mock_route_message: pass message_id and set _pending_message_ids
  on session_pool_integration (was: drop message_id in **kwargs)
- Pre-initialize _pending_message_ids/_pending_message_metadata as
  real dicts on the AsyncMock to avoid auto-created Mock attributes

New tests (test_duplicate_message_id.py):
- test_mock_append_message_raises_on_duplicate: verifies mock catches
  duplicates (meta-test for test infrastructure)
- test_duplicate_assistant_message_does_not_raise: verifies the fix
  handles double-write gracefully
- test_different_messages_both_stored: non-duplicate messages still work
- test_user_message_then_assistant_message_no_error: mixed messages OK
- test_route_message_sets_pending_message_ids: verifies mock passes
  message_id through to _pending_message_ids

Closes #229

* Revert "fix(opencode): handle duplicate message ID in append_message_to_session (#229)"

This reverts commit 5a4cd4ad6c773af73174c7f76a430ead1d3d6225.

* fix: team_add_member bugs — agent field, blackboard key sanitization, atomicity

- register_member now accepts agent param, stores actual agent type
  (was storing member_name as agent — caused team_status to show wrong agent)
- Sanitize blackboard keys with re.sub for non-ASCII names (中文/连字符)
- Blackboard write is now non-fatal (try/except + warning log)
  — member creation succeeds even if blackboard audit trail fails
- team_create also passes agent type to register_member
- team_remove_member blackboard key also sanitized

* test: tighten team_add/remove_member tests — 8 new cases, reusable helper

- Extract _make_add_member_setup() helper (eliminates ~15 lines duplication)
- Tighten test_team_add_member_success: assert agent field in team state,
  session_id, team_member_sessions metadata
- Tighten test_team_add_member_with_notify: assert notify targets exclude
  lead and new member
- Tighten test_team_remove_member_success: assert blackboard write
- New: non-ASCII name (中文), hyphen name, agent field correctness,
  max_members exceeded, agent not in registry, metadata update,
  non-ASCII removal, notify exclusion verification
- 154 tests pass (was 146)

* fix: ghost member on send_message + shutdown_request marks member offline

Bug #1: send_message to non-existent member created phantom entry in
team state because bounds check (turn_count increment) ran before the
get_member_session_id check. Fix: move member existence check before
bounds check so non-existent members are rejected without touching state.

Bug #2: shutdown_request closed the session but didn't update team state,
so team_status still showed the member with a stale session_id. Fix:
clear session_id and set status='shutdown' in team state after closing.

New tests:
- test_send_message_to_nonexistent_member_no_phantom
- test_shutdown_request_marks_member_offline
156 tests pass.

* fix: BUG-001 team_status missing dynamic members + NOTE-001 delete_blackboard

BUG-001: team_status didn't show dynamically added members when
team_mode_config was None in the per-turn AgentContext. Root cause:
_get_team_state fell back to tempfile.gettempdir() instead of the
base_dir used by team_create. Fix: team_create stores base_dir in
session.metadata['team_base_dir'], and _get_team_state reads it
from there when team_mode_config is None.

NOTE-001: delete_blackboard on non-existent key now returns 'not found'
with a list of available keys (consistent with read_blackboard).

2 new regression tests:
- test_team_status_shows_added_member_no_team_mode_config
- test_delete_blackboard_nonexistent_key_returns_not_found
158 tests pass.

* fix: task_update with invalid task_id returns friendly error

Was raising FileNotFoundError [Errno 2] from FileTeamState.update_task.
Now catches FileNotFoundError/OSError and returns 'Task not found: {task_id}'.

Merge from origin/main included.
159 tests pass.

* fix: allow load_session for closed sessions (history replay)

ACP load_session was rejecting sessions with status='closed' by raising
Resource not found. This prevented users from viewing team member
conversation history after team_delete closed all member sessions.

Now closed sessions are loaded via resume_session for read-only history
replay. The conversation history is accessible but the session won't
accept new prompts (it's still closed in the SessionPool).

Root cause: team_delete calls close_session on all members, setting
their status to 'closed' in the store. When users later click on a
member's agent card in the UI, load_session found status='closed' and
rejected the request.

159 tests pass.

* refactor: team tools return XML-wrapped content instead of JSON

- read_blackboard: returns <blackboard version="N" written_by="...">content</blackboard>
- task_list: returns <task_list><task id="..." status="...">subject: desc</task></task_list>
- task_update: returns <task id="..." status="...">subject: desc</task>
- list_blackboard: returns <blackboard_keys>key1\nkey2</blackboard_keys>

Benefits for LLM consumption:
- No JSON escaping/escaping overhead
- Content can be markdown, YAML, or any text
- Metadata in XML attributes, content in body
- Cleaner token usage (no {"value":{"text":"..."}} wrapping)

Updated 14 test assertions across 5 test files.
159 tests pass.

* feat: team tools return pydantic-ai ToolReturn, add source="team" for UserMessageInsertedEvent

- All 14 team tools return ToolReturn(return_value=...) instead of plain strings
- Add "team" to UserMessageInsertedEvent.source Literal type
- send_message() accepts source parameter, passed through _route_message
- All team send_message calls pass source="team" + meta={from, team_id}
- ACP/OpenCode handlers prefix team messages with [Team · {from_member}]
- QUEUE mode None return disambiguated via session existence check
- send_message urgent defaults to True (STEER) for all team communication
- All 159 tests pass, ruff clean

* feat: team member awareness + team_status runtime state + task association

- broadcast_on_create config (default True): auto-broadcast to all existing
  members (except lead) when team_add_member creates a new member
- team_create: inject member roster into initial prompt so members know
  their teammates from the start
- team_add_member: new member also receives current member roster
- team_status: show runtime state per member (online/busy/offline/closing),
  turn_count (X/Y), inbox message count, and associated incomplete tasks
- Update test_team_add_member_success for broadcast_on_create behavior
- All 159 tests pass, ruff clean

* refactor: merge broadcast_notice into notify parameter for team_add_member

- Remove redundant broadcast_notice parameter
- notify is now embedded in the auto-generated broadcast message:
  [团队通知] 新成员 'xxx' (agent=yyy) 已加入团队。{notify}当前成员: ...
- Update tests to use 'in' check instead of exact match for notify content
- All 159 tests pass

* feat: reuse agent display_name for team member names

- team_add_member: when name is empty, fall back to agent's display_name
  (from AgentRegistry), then to agent name
- team_create: same fallback for members with empty name field
- All 159 tests pass, ruff clean

* fix: propagate team member name to agent display_name

- team_create: set member_agent._display_name = member['name'] after
  child session creation
- team_add_member: same — member name becomes the agent's display_name
- Revert previous fallback logic (name ← display_name), correct
  direction is name → display_name
- Protocol frontends (ACP, OpenCode) now show the team member's display
  name instead of the underlying agent name
- All 159 tests pass, ruff clean

* refactor: shorten task_id and team_id to 8-char hex prefix

- task_id: task_{8 hex chars} (was full UUID)
- team_id: team_{8 hex chars} (was full UUID)
- Matches xeno-agent background task pattern (bg_XXXXXXXXXXXX)
- Shorter IDs are more LLM-friendly in tool results and references
- All 159 tests pass

* refactor: team message XML wrapper in body, remove protocol handler prefix

- send_message (direct): body wrapped as <team-message from="xxx" type="private">
- send_message (broadcast): body wrapped as <team-message from="xxx" type="broadcast">
- team_add_member broadcast: notice wrapped as <team-message from="xxx" type="broadcast">
- Revert ACP/OpenCode handler [Team ·] prefix — rendering controlled by body content
- All 159 tests pass, ruff clean

* refactor: use English for all team message templates

- broadcast_on_create notification: English format with clear line breaks
- team_create/team_add_member roster header: ## Team Members
- notify docstring: English example
- All 159 tests pass, ruff clean

* style: wrap member names, agent names, and roles in backticks

- Roster lines: `name` (agent=`agent`, role=`role`)
- Broadcast: New member `name` (`agent`) joined
- team_status: `name` (agent=`agent`, status=`status`, ...)
- All 159 tests pass, ruff clean

* feat: restrict task_create to lead-only

- task_create now requires team_role=lead, returns error for members
- Add task_create to _LEAD_ONLY_TOOLS frozenset for prepare_tools filtering
- Members no longer see task_create in their tool list (7 universal tools remain)
- Fixes edge case #10: members self-creating duplicate tasks
- Update all affected tests to use lead metadata for task_create calls
- All 159 tests pass

* refactor: remove urgent param, use notice_delivery_mode config, clean docstrings

- Replace `urgent` parameter on send_message with config-level
  `notice_delivery_mode: Literal["steer", "queue"]` (default "steer`)
- Remove `auto_urgent` config field — all team communication uses
  the configured delivery mode
- Add `_notice_mode` property to resolve DeliveryMode from config
- Remove `ctx` from all tool docstrings (pydantic-ai internal,
  not relevant to the model)
- Use `Annotated[type, Field(description=...)]` for parameter
  descriptions instead of docstring Args sections — eliminates
  duplicate info between tool description and JSON schema
- Fix E501 lint error in test_unit_capability.py
- Update test_session_close_then_load for closed-session history
  replay behavior (ec8d706af)
- Add D417 per-file-ignore for team_comm_capability.py (ctx param
  intentionally omitted from docstrings)

159 tests pass, ruff clean.

* feat: write_blackboard append mode + value format description

- Add `mode` parameter to write_blackboard tool: "overwrite"
  (default) or "append". Append concatenates new value to existing
  text with newline separator — useful for accumulating findings
  or logs across multiple writes.
- Update `value` Field description to note supported formats:
  inline JSON, Markdown, or plain text.
- Add tests for append mode and append-to-empty-key scenario.

161 tests pass, ruff clean.

* fix: defer member cleanup when lead or members have active runs

_schedule_member_cleanup used last_active_at as the sole idle signal,
but last_active_at is only updated by send_message() — not during
model calls or tool execution.  A long-running turn could make the
lead appear idle after 300s, triggering premature closure of member
sessions that were still actively processing.

Add current_run_id checks for both the lead and all members before
closing.  If any session has an active run, cleanup is deferred to
the next poll cycle.

* fix(opencode): populate AssistantMessage tokens/cost from EventProcessor context

StreamCompleteEvent finalization was called BEFORE adapter.convert_event(),
which runs the EventProcessor that populates ctx.input_tokens/output_tokens
from msg.usage.  The result: _finalize_assistant_time broadcast
MessageUpdatedEvent with AssistantMessage.info.tokens = Tokens() (all zeros),
so the OpenCode TUI showed no token usage.

Fix: move finalization after adapter.convert_event() and write the
now-populated token/cost values onto info before broadcasting.

Root cause traced end-to-end:
  API returns usage ✅ → pydantic-ai parses ✅ → ChatMessage.usage ✅
  → EventProcessor updates ctx ✅ → AssistantMessage.info.tokens = 0 ❌

The old _wait_and_finalize (message_routes.py:801) did update info.tokens
but has been dead code since the EventBus path replaced it.

* feat: team_create prompt param, watch mode for list_blackboard/team_status

- team_create: add optional `prompt` parameter for initial member
  task instructions; improve `members` Field description with
  JSON example
- list_blackboard: add `watch` (bool, default False) and `timeout`
  (int, default 300s) params. When watch=True, polls for blackboard
  key changes and returns when new keys appear/disappear or timeout
  expires
- team_status: add `watch` and `timeout` params. When watch=True,
  polls team state file mtime for changes (member status updates,
  task changes) and returns when changes detected or timeout expires
- Add 4 new tests: team_create with prompt, list_blackboard watch
  timeout, list_blackboard watch detects change, team_status watch
  timeout

167 tests pass, pre-commit all passed.

* feat: task_update note field for progress tracking

- Add `note` parameter to task_update: records last update note,
  timestamp, and author on the task
- task_list now displays `last_note` when present
- Enables members to report progress without sending separate messages

167 tests pass, ruff clean.

* feat: prompt members to use task_update(note=...) for progress

- Protocol template: update guideline to mention task_update(note=...)
- team_create with prompt: append reminder to report progress via
  task_update(note="...")
- team_add_member with prompt: same reminder appended

167 tests pass, ruff clean.

* feat: notice_role config for system/user message injection

- Add `notice_role: Literal["user", "system"]` config field
  (default "user")
- When notice_role="system" and notice_delivery_mode="steer",
  team notifications are wrapped in SystemPromptPart and injected
  as system messages via PydanticAI's enqueue()
- No agentpool core changes needed — SystemPromptPart is passed
  as content directly from team_comm_capability
- Initial prompts (QUEUE mode) always use user role

167 tests pass, ruff clean.

* fix(opencode): handle SystemPromptPart in user message event processing

When team notifications use notice_role=system, the content is
[SystemPromptPart(content=...)]. The OpenCode event processor's
_process_user_message_inserted only handled str and dict items in
list content — SystemPromptPart (a pydantic model) was silently
skipped, causing the TUI to show empty messages.

Fix: extract .content from any object with a string content attribute
(covers SystemPromptPart and other ModelRequestPart types).

167 tests pass, ruff clean.

* fix: wrap SystemPromptPart in ModelRequest for enqueue compatibility

SystemPromptPart passed directly to enqueue() was being gathered
into a UserPromptPart by PendingMessageDrainCapability, causing
assert_never in OpenAI model mapper.

Fix: wrap in ModelRequest(parts=[SystemPromptPart(...)]) — enqueue()
keeps complete ModelRequest objects as-is instead of coalescing them
into user prompt content.

Also update event processor to extract text from ModelRequest.parts
for TUI display.

167 tests pass, ruff clean.

* feat(team-mode): merge shutdown_request into team_remove_member, add blackboard pagination and member work summary

- Delete old shutdown_request (soft shutdown that leaked max_members quota
  by keeping members in the dict with status='shutdown')
- Rename team_remove_member to shutdown_request (hard remove: closes
  session, removes from members dict, cleans up session metadata, writes
  audit to blackboard)
- Add line-based pagination to read_blackboard: limit (default 200),
  offset (0-indexed), context (center around a line number). Returns
  list[str] instead of joined string. Appends truncation hint when more
  lines exist.
- Inject work-status summary into team_add_member roster so new members
  know what existing members are working on (in_progress / completed /
  no active work)
- Update AGENTS.md tool table
- Update all affected tests (192 pass)

* feat(team-mode): add unfinished task reminder harness for team members

Add after_run hook in TeamCommCapability that checks for in_progress
tasks when a member agent's run completes. If unfinished tasks are
found, routes a reminder message to the member's own session via
session_pool.send_message (QUEUE mode). Limited to 1 reminder per
session to avoid infinite loops. Skipped for lead agents and during
session shutdown.

Modify shutdown_request to check for unfinished tasks before closing
a member's session. If any in_progress tasks are found, the ToolReturn
includes a warning telling the lead to update task status or reassign.

Add _get_unfinished_tasks static helper to avoid code duplication
between after_run and shutdown_request.

7 new unit tests covering: reminder sent for unfinished tasks, no
reminder for lead, no reminder when session closing, no duplicate
reminders, no reminder when all tasks completed, shutdown warning
with unfinished tasks, shutdown no warning when tasks completed.

* feat(team-mode): add watch_task_ids to list_blackboard and team_status watch

Both tools now accept an optional watch_task_ids parameter. When non-empty,
the watch loop monitors specific task file mtimes instead of general state
changes, returning as soon as any watched task is modified. When empty or
None, the existing behavior is preserved (any change ends the watch).

- Add _snapshot_task_mtimes static helper for task file mtime detection
- list_blackboard: watch_task_ids filters to specific task changes
- team_status: watch_task_ids filters to specific task changes
- 5 new tests covering timeout, detection, and unrelated-change filtering

* feat(team-mode): add owner parameter to task_create

task_create now accepts an optional owner parameter to assign a team
member as the task owner at creation time, eliminating the need for a
separate task_update call.

* feat(team-mode): add max_watch_timeout config and timeout<=0 no-limit semantics

- Add max_watch_timeout field to TeamModeConfig (default 120s)
- list_blackboard and team_status: timeout<=0 means no user limit, uses
  config max; timeout>0 is capped by config max via min(timeout, max)
- 3 new tests: timeout=0 uses config max, timeout capped by config,
  team_status timeout=0 uses config max

* fix(storage): enable SQLite WAL mode and busy_timeout to prevent database lock errors

Concurrent session writes to the shared SQLite database were causing
'sqlite3.OperationalError: database is locked' errors. Add PRAGMA
settings on every new SQLite connection:

- journal_mode=WAL: allows concurrent readers with a single writer
- busy_timeout=30000: writers wait up to 30s for the lock instead
  of failing immediately
- synchronous=NORMAL: safe with WAL, reduces fsync overhead

* fix(team-mode): event-driven ephemeral member cleanup instead of 5s polling

Replace 5-second polling loop with complete_event.wait() on the
RunHandle for event-driven completion detection. Write team state
file BEFORE closing session so team_status(watch=True) detects the
change immediately. Add broad exception handling to prevent silent
task death when close_session fails.

* feat(team-mode): notify lead when member crashes via unified message routing

When a team member's run fails with an exception, the lead (parent
session) now receives a concise notification through _route_message
with source="team". This goes through the same unified message path
as member-initiated send_message, so it appears in the lead's
conversation history and the LLM can act on it in the next turn.

Previously, when a member crashed, the lead had no notification at
all — RunErrorEvent was published only to the member's own EventBus,
which the lead never subscribed to. The lead could only discover the
crash by manually polling team_status.

The notification is best-effort: if the lead session is unavailable
or closed, the notification is silently skipped. Normal member
completion (where the member calls send_message to the lead) is
unaffected — no duplicate notification.

* feat(team-mode): make idle_timeout and poll_interval YAML-configurable

Add idle_timeout (default 600s) and poll_interval (default 30s) fields
to TeamModeConfig so users can tune team member cleanup timing in YAML:

  team_mode:
    enabled: true
    idle_timeout: 600
    poll_interval: 15

Previously these were hardcoded class attributes (300s/30s) on
TeamCommCapability with no YAML config path. The _schedule_member_cleanup
method was converted from @staticmethod to instance method to access
self._config. Tests updated to pass values via manifest instead of
monkeypatching class attributes.

* feat(team-mode): push notifications for task assignment and unblock

Add _notify_member helper to TeamCommCapability that sends a
best-effort system notification to a team member's session via
session_pool.send_message().

Three notification triggers:
1. task_create with owner set → notify the assigned member
2. task_update with new owner → notify the newly assigned member
   (skipped when task is already completed)
3. task_update with status=completed → find all downstream tasks
   whose blocked_by contains the completed task_id, check if they
   are now fully unblocked (is_unblocked), and notify their owners

Notifications use the existing notice_delivery_mode (steer/queue)
from TeamModeConfig and are wrapped as <team-message type=
task_notification>. Self-notifications are skipped. All failures
are logged as warnings and do not block the tool return.

* fix(team-mode): use team_member_name for task_update permission check

The permission check in task_update compared the task owner against
self._agent_name (the YAML agent name, e.g. "translator"), but task
owners are set using team member names (e.g. "artisan_23830").  This
mismatch caused permission-denied errors when members tried to update
their own tasks.

Fix: extract team_member_name from session metadata (with _agent_name
fallback) and use it for both the ownership check and the updated_by
field.  Update test to assign tasks using the member name.

* fix(team-mode): check session.is_closing before chaining queued prompts

When a team is closed, member sessions' ProtocolChannel is closed.
But _consume_run could still pick up queued prompts from prompt_queue
and try to start a new turn, causing RuntimeError: ProtocolChannel is
closed; cannot publish.

Fix: check session.is_closing inside the _request_lock block before
creating a new RunHandle for chained prompts.

* feat(team-mode): allow members to use task_create for subtasks

Remove task_create from _LEAD_ONLY_TOOLS so members can see and use
it.  The runtime permission check inside task_create already blocks
non-lead members from creating top-level tasks (parent_id=None),
while allowing subtask creation (parent_id set).  This enables more
autonomous collaboration — members can break down their work into
subtasks without lead involvement.

* fix(team-mode): suppress member crash notification during normal shutdown

When team_delete or shutdown_request closes member sessions, the
ConsumeRun exception handler would fire (ProtocolChannel closed,
RuntimeError) and notify the lead of a member crash.  This is
actually a normal shutdown, not a crash.

Fix: check session.is_closing before calling _notify_lead_of_member_crash.
If the session is being closed, the exception is expected and the
lead should not receive a crash notification.

* refactor(team): switch child session creation from RunLoopDelegationService to SessionPool

Replace agent_ctx.delegation.create_child_session() (Path A) with
direct SessionPool.create_child_session() calls (Path B) in
team_comm_capability.py for both team_create and team_add_member.

The new _create_member_session() helper:
- Uses SessionPool.create_child_session() which generates ses_ prefixed
  sortable IDs (same as session ID generation)
- Eagerly registers the agent via get_or_create_session_agent()
- Emits SpawnSessionStart for protocol server discovery

Remove create_child_session() from:
- RunLoopDelegationService (concrete implementation)
- DelegationService Protocol

This eliminates the dual-path problem where RunLoopDelegationService
created sessions with a different code path than AgentRunContext,
missing agent registration and done_event setup.

* fix(identifiers): switch session IDs to descending order to match OpenCode TUI

OpenCode's SessionID.create() uses descending() (bitwise NOT of
timestamp), so newer sessions have lexicographically smaller IDs.
The TUI's children() memo sorts by session.id ascending, and
moveChild(1)=Next moves toward index 0 (newest). With agentpool's
ascending IDs, this was reversed: Next went to oldest, Previous to
newest.

Changed generate_session_id() from ascending(session) to
descending(session), and updated session_routes.py's two direct
ascending(session) calls to descending(session) for consistency.

Updated test_generate_session_ids_are_sortable to assert descending
order. The ascending() function itself is unchanged — it's still used
for message, part, and other non-session ID types.

* fix: sync created_at_ns with session ID timestamp for consistent ordering

Session ID and created_at_ns were captured at different times
(generate_session_id() vs SessionState construction), causing
millisecond gaps that flip ordering for rapidly-created sessions.

Add extract_timestamp_ms() to decode the timestamp embedded in
ascending/descending IDs. After SessionState creation, override
created_at_ns and last_active_at_ns with the session ID's timestamp,
ensuring time.created order always matches session ID lexicographic
order.

* deps: upgrade pydantic-ai-slim from 2.9.0 to >=2.12.0

Enables EnqueuedMessagesEvent support (available since v2.12.0).
Fix ToolReturn import from pydantic_ai.messages instead of
pydantic_ai.tools (re-export removed in newer versions).
Move RunContext/ToolDefinition into TYPE_CHECKING block.

* fix: default request_limit to None (unlimited) for native agents

PydanticAI's UsageLimits defaults request_limit to 50, which is too
low for agents with many tool calls. When no usage_limits are
explicitly configured, default to request_limit=None (unlimited).

* fix(team-mode): add 2ms delay between member session creations

Ensures each member session gets a distinct time.created (millisecond
precision) when team_create creates multiple members in a tight loop.
SQLite WAL store operations complete in sub-millisecond time, so
without this delay all members get the same time.created, causing
non-deterministic sort order in OpenCode TUI subagent numbering.

* fix(opencode): override stored created_at with session ID timestamp

session_data_to_opencode() now uses extract_timestamp_ms(session_id)
to derive time.created when the session ID is ses_ format. This fixes
old sessions persisted before the created_at_ns sync fix — their
stored created_at came from get_now() (separate wall-clock call),
which can differ from the session ID's embedded timestamp by enough
to cause sort mismatches in OpenCode TUI subagent numbering.

* fix(team-mode): add 10ms delay in _create_member_session for distinct timestamps

Move delay from team_create loop into _create_member_session() to
cover all paths (team_create and team_add_member). 10ms ensures
each session gets a distinct time.created (millisecond precision),
preventing sort mismatches in OpenCode TUI subagent numbering.

* fix(team-mode): randomize delay 5-20ms in _create_member_session

* fix(team-mode): increase delay range to 15-50ms in _create_member_session

* fix(opencode): use per-step delta for AssistantMessage.tokens

Using cumulative values caused the TUI sidebar to show extremely large
token counts that grew quickly on each step, because each LLM request
re-sends the entire system prompt + conversation history.

Switch to per-step delta so the sidebar shows the actual cost of each
step (e.g., input=55460, output=1517) instead of the running
cumulative total (e.g., input=324969).

* fix(team-mode): add debug log for _create_member_session delay

* fix(team-mode): use asyncio.Lock instead of random delay for session serialization

Replace the 15-50ms random sleep with an asyncio.Lock around
create_child_session() calls. Concurrent tool invocations from
PydanticAI could fire multiple _create_member_session() in parallel,
making both delays start simultaneously — the generate_session_id()
calls still landed in the same millisecond. A lock guarantees true
serialization.

* fix: resolve ruff lint errors from origin/main merge

- Remove 50 unused # noqa: BLE001 directives (RUF100)
- Move None to end of type union in 4 locations (RUF036)
- Parenthesize implicit string concatenation in collections (ISC004)

* feat(team-mode): collab-flow improvements — per-member instructions, handoff, batch, progress

Implements the team-mode-collab-flow OpenSpec change with 9 design decisions:

- MemberSpec.instructions: per-member role text injected as ## Your Assignment
- Protocol template rewritten with explicit channel boundaries (Tasks/Blackboard/Messages)
- task_update note → technical_note (disambiguate from communication)
- Task handoff: handoff_to + handoff_context_keys for one-step handoff with notifications
- Enhanced dependency notifications: <team-message type=dependency_resolved> + self-skip
- send_message persist_to_blackboard: eliminate message-vs-blackboard dilemma
- task_create_batch: atomic batch creation with #N and symbolic id references
- Progress tracking: progress_current/progress_total with auto-complete on status=completed
- Owner visibility: mine_only filter, owner summary, actionable ownership errors

15 tools total (was 14): 9 universal + 6 lead-only (added task_create_batch)

298 tests pass (91 new). ruff check + format clean. RFC-0055 updated.

* feat(team-mode): enrich spawn events and ToolPart with team context

Team-mode display enrichment (tasks 6-9 of enhance-opencode-display):

- team_comm_capability.py: SpawnSessionStart.metadata includes team context
- team_comm_capability.py: team_create sets team_role='lead' on lead session
- opencode_message_bridge.py: Team members get 'Team ·' prefix in subagent_type
- opencode_event_bridge.py: Team member session titles include team name and role
- 8 new unit tests for team display enrichment

All 306 team-mode tests pass.

* fix(opencode): use display_name in _update_parent_toolpart and _update_parent_toolpart_error

The update/error methods were overwriting ToolPart state with source_name
instead of display_name, causing the card to revert from 'Lead' to
'Coordinator' when the subagent completed or errored.

* fix(opencode): update child session title after ensure_session

ensure_session() does not update the title when the session already
exists (fast path or store-first path). The child session is created
by SessionPool.create_child_session() before SpawnSessionStart is
emitted, so _ensure_child_session_visible's title parameter was
silently ignored.

Now we explicitly update session.title and broadcast SessionUpdatedEvent
when the title differs from the desired value.

* fix(team-mode): correct tool_call_id, spawn_mechanism, and display propagation

- team_comm_capability: pass tool_call_id from ctx to SpawnSessionStart
  instead of hardcoded empty string (fixes ACP/OpenCode tool call correlation)
- team_comm_capability: change spawn_mechanism from 'spawn' to 'task'
  (team members are async via DeliveryMode.QUEUE)
- opencode_message_bridge: set background=True in ToolPart metadata when
  spawn_mechanism=='task' (TUI shows '(background)' suffix)
- opencode_message_bridge: simplify ToolPart card description to
  '{role} in '{team_name}'' (remove redundant name repetition)
- opencode_event_bridge: use source_name (ASCII) in @xxx subagent pattern
  instead of display_name (may be non-ASCII, breaks TUI regex)
- opencode_event_bridge: set mode to team_member_name for assistant footer
- tests: update assertions for new spawn_mechanism, background flag,
  session title pattern, and simplified card description

* fix(opencode): remove redundant tool name from ToolState title

TUI renders '↳ {Locale.titlecase(tool)} {title}', but title was
'Running {tool_name}' / 'Completed {tool_name}', causing duplicate
tool name display (e.g. 'Segment_scan Completed segment_scan').

Changed title to just 'Running' / 'Completed' in converters.py
and event_processor.py (4 call sites).

* fix(team-mode): prefer semantic member names in team_add_member description

* fix: ProtocolChannel closed errors, CI type ignores, flaky benchmark, blackboard namespace keys

- Add _safe_publish() helper on RunHandle to catch ProtocolChannel closed
  RuntimeError during session shutdown, preventing cascade errors
- Remove 5 unused # type: ignore[assignment] comments in team_comm_capability.py
- Relax benchmark threshold from 3x to 5x for CI runner variance
- Set session_state.metadata = {} in test_mode_consistency mocks
- Fix list_blackboard glob→rglob to return namespace keys with '/' separators
- Add test_list_blackboard_returns_namespace_keys unit test

* fix(team-mode): TDD fixes for format_task_xml crash, state.json race, blackboard cleanup, per-instance session lock

- format_task_xml: move parts list init outside if block to prevent
  UnboundLocalError when progress fields are None
- register_member: wrap state.json read-modify-write in FileLock to
  prevent data loss under concurrent registration
- write_blackboard: delete oversized file when max_size_mb exceeded so
  subsequent reads return 'Key not found' instead of stale data
- _create_session_lock: move from module-level global to per-instance
  asyncio.Lock so different teams don't serialize session creation

All 271 unit tests pass, ruff lint+format clean.
Million-mo added a commit that referenced this pull request Aug 3, 2026
#1 tool_call_id: QuestionCapability._question already uses replace() to
propagate tool_name/tool_call_id/tool_input — verified with L2 test.
#2 telemetry: add @logfire.instrument to QuestionCapability._question;
background_task modules already instrumented.
#3 state cleanup: after_run() evicts _session_states and _ephemeral_states,
shuts down batcher and task manager.
#5 queued cancel: pending cancel path fires on_completed before
completion_event.set().
#6 flush exception: _flush catches broad Exception, marks delivered
regardless of success/failure.
#7 timeout message: CancelledError handler checks task.status ==
'timed_out' before choosing message.
#8 private API: guard pydantic_ai._agent_graph import with try/except
and helpful error message.
#9 L2 test: add test_question_tool_propagates_tool_call_id_to_agent_context.
#10 error contract: _build_definition raises ValueError with descriptive
message for missing name or non-dict input.
Nit: remove DEBUG_TASK_MGR prefix, type coro as Coroutine, fix test names.
Million-mo added a commit that referenced this pull request Aug 4, 2026
…Capability from xeno-agent (#346)

* feat(capabilities): add QuestionCapability with YAML schema override support

Move question tools into a proper AbstractCapability that accepts
args.schemas and args.enabled_tools, mirroring BackgroundTaskCapability.
Replaces the bare QuestionTools entry point so consumers can customize
LLM-facing parameter descriptions via YAML schema files without writing
their own capability wrapper.

* test(question): migrate question tool tests from xeno-agent

Move 51 question tool unit tests (40 for question_for_user + 11 for
ask_followup_question) from xeno-agent to agentpool. Tests now import
from agentpool_toolsets.builtin.question_tools instead of xeno_agent.

Two assertions adjusted to match agentpool's actual implementation:
- Error message regex: 'questionnaire' → 'questions' (agentpool naming)
- ask_followup_question metadata: dropped suggestion_attributes check
  (agentpool's _format_followup_response doesn't emit this field)

Also add RUF001 per-file ignore for CJK fullwidth punctuation in test data.

* feat(question): merge simple question tool into QuestionCapability

Add a 'question' tool to QuestionCapability that replicates the legacy
QuestionTool behavior (simple prompt + optional response_schema via MCP
Elicit). This unifies all user-interaction tools under one capability:
  - question_for_user: XML multi-question questionnaire
  - ask_followup_question: single question with <suggest> options
  - question: simplest single-question (replaces QuestionTool)

Mark QuestionToolConfig (tools: [{type: question}]) as deprecated,
directing users to capabilities: [{type: question}].

Add 3 new tests covering the question tool: default-enabled, enabled
alone, enabled via schemas.

* feat(capabilities): add BackgroundTaskCapability migrated from xeno-agent

Migrate the complete BackgroundTaskCapability implementation to agentpool:
- capability.py: full lifecycle management (task, background_output,
  background_cancel, steer_task tools)
- manager.py: BackgroundTaskManager with concurrent task execution,
  cleanup, and session isolation
- notification.py: NotificationBatcher for debounced completion notifications
- types.py: BackgroundTask, SessionTaskState dataclasses
- utils/tool_schema.py: YAML schema loading and LLM-facing schema override

Includes 277 tests (unit + integration + resource provider) covering
lifecycle, concurrency, error propagation, notification batching,
history isolation, and cancellation regression.

Config schema files (task.yaml, background_output.yaml,
background_cancel.yaml, steer_task.yaml) provide default LLM-facing
parameter descriptions.

Entry point 'background_task' registered in pyproject.toml.

* fix(ci): resolve ruff format, mypy, and flaky test failures

Three CI failures fixed:

1. ruff format: question.py ternary expression reformatted

2. mypy (16 errors → 0):
   - tool_schema.py: cast yaml.safe_load/json.loads results to
     OpenAIFunctionDefinition, construct TypedDict with explicit
     key-value pairs instead of ** unpacking
   - question.py: add ToolResult return annotations to tool wrappers
   - manager.py: re-read task_model.status into locals after await
     to prevent mypy narrowing from concurrent status changes
   - capability.py: type session_pool as SessionPool | None,
     fix delivered bool assignment from followup() str|None return,
     remove dead config.type == 'team' comparison (agents dict never
     contains team configs), rename shadowed task_model variable

3. Flaky tests (8 failures): replace fixed asyncio.sleep(0.8) with
   _wait_until_called() polling helper in 3 test files. The fixed
   sleep was too tight under CI load — debounce timers fire late
   when the event loop is busy with parallel workers.

* fix(ci): replace anyio TaskGroup with asyncio.ensure_future in NotificationBatcher

Root cause: loop.call_later callbacks run in a separate contextvars
context where anyio's sniffio async-library detection fails with
AsyncLibraryNotFoundError. This prevented _schedule_flush from
calling tg.start_soon, so _flush never executed and deliver_callback
was never invoked — all 25 batcher tests + 8 notification tests
failed in CI.

Fix: replace anyio.create_task_group/start_soon with
asyncio.ensure_future for scheduling flush coroutines. Keep
anyio.CancelScope and anyio.fail_after for timeout protection
(these don't require sniffio context). Track flush tasks in a
set[asyncio.Task] and cancel/await them in shutdown.

Also restore source_type team detection by checking config.type
via str() cast (mypy-safe for AnyAgentConfig union that doesn't
include team types at the type level, but mocks provide type='team'
at runtime).

* refactor(question): unify question tools into single question tool

Merge ask_followup_question, question_for_user, and question into one
unified question tool per reviewer feedback. The question_for_user
implementation (richest, supports multi-question XML with enum/multi/
input types) is retained as the canonical implementation, renamed to
question. ask_followup_question (legacy compat) and the simple question
tool are removed.

Changes:
- question_tools.py: remove ask_followup_question + _format_followup_response,
  rename question_for_user to question
- question.py: simplify QuestionCapability to expose only question
- Update all tests, docs, and tool name references

* fix(review): address opencode-agent review findings

#1 tool_call_id: QuestionCapability._question already uses replace() to
propagate tool_name/tool_call_id/tool_input — verified with L2 test.
#2 telemetry: add @logfire.instrument to QuestionCapability._question;
background_task modules already instrumented.
#3 state cleanup: after_run() evicts _session_states and _ephemeral_states,
shuts down batcher and task manager.
#5 queued cancel: pending cancel path fires on_completed before
completion_event.set().
#6 flush exception: _flush catches broad Exception, marks delivered
regardless of success/failure.
#7 timeout message: CancelledError handler checks task.status ==
'timed_out' before choosing message.
#8 private API: guard pydantic_ai._agent_graph import with try/except
and helpful error message.
#9 L2 test: add test_question_tool_propagates_tool_call_id_to_agent_context.
#10 error contract: _build_definition raises ValueError with descriptive
message for missing name or non-dict input.
Nit: remove DEBUG_TASK_MGR prefix, type coro as Coroutine, fix test names.

* chore: remove list_available_nodes tool (legacy, Leoyzen feedback #345)

- src/agentpool_toolsets/builtin/subagent_tools.py: remove
  list_available_nodes method + create_tool registration
- src/agentpool_config/toolsets.py: SubagentToolName Literal now
  only accepts 'task'; docstring updated
- tests/toolsets/test_tool_filtering.py: update assertions
- tests/toolsets/builtin/test_as_capability.py: update assertions
- tests/servers/acp_server/test_claude_acp_toolset_integration.py:
  update assertion
- tests/tools/test_runcontext.py: remove prompt referencing
  list_available_nodes (test was already xfail)
- docs/how-to/advanced/acp-integration.md: update docs
- docs/how-to/servers/mcp-server.md: update docs

The tool was legacy code; Leoyzen noted agents list is now
injected directly into system prompt.

* fix: ruff format toolsets.py (single-entry Literal syntax)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants