Skip to content

fix(agents): drive inline agent sessions via run_agent_session, not the phantom create_agent_session#341

Merged
OBenner merged 2 commits into
developfrom
claude/sharp-edison-d7f96a
Jun 14, 2026
Merged

fix(agents): drive inline agent sessions via run_agent_session, not the phantom create_agent_session#341
OBenner merged 2 commits into
developfrom
claude/sharp-edison-d7f96a

Conversation

@OBenner

@OBenner OBenner commented Jun 13, 2026

Copy link
Copy Markdown
Owner

Problem

ClaudeSDKClient (from claude_agent_sdk) has no create_agent_session method, so every inline call site raised 'ClaudeSDKClient' object has no attribute 'create_agent_session' at runtime. The shared generator boilerplate (agents/_generator_base) was fixed in #339, which explicitly called out these remaining inline call sites as a follow-up. This is that follow-up.

Fix

Each inline session now drives the client through run_agent_session() (agents/session.py) inside async with client — the same session API the planner/coder/qa phases use — and treats a returned status == "error" as failure. The agent role prompt is delivered as the leading message (the SDK client only carries the generic base system prompt).

Call site File
run_documentation_generator_session documentation_generator.py
run_migration_assistant migration_assistant.py
run_test_generator_session (writer and coverage-improvement sessions) test_generator.py
run_agent_session (isolated-subprocess wrapper) agent_subprocess.py

Notes:

  • agent_subprocess imports the helper under an alias (run_agent_session as run_sdk_session) so it does not shadow that module's own run_agent_session() wrapper; the JSON output shape consumed by run_agent_session_isolated is preserved.
  • test_generator's two sessions each run in their own async with client block (create_client returns an options-based, reconnectable client). The improvement message is self-contained and the agent re-reads its earlier output from disk.
  • Agent types (documentation_generator, migration_assistant, test_generator) are already registered in AGENT_CONFIGS; agent_subprocess receives its type from the caller (coderrun_agent_session_isolated).

Tests

  • New: tests/test_inline_generator_sessions.py mirrors tests/test_fixture_generator_session.py — a fake async-context-manager client with no create_agent_session attribute, agents.session.run_agent_session patched as an AsyncMock, asserting it is awaited, the client is entered/exited, and the role prompt leads the message. Covers all four modules plus error propagation and the test_generator two-session (writer + improvement) path.
  • Updated: existing suites that mocked the phantom method now drive the shared session helper instead — tests/test_migration_assistant.py, tests/test_test_generation.py, tests/integration/test_migration_workflow.py, and a dead-mock cleanup in tests/e2e/test_qa_test_generation_flow.py.

Rebased onto current develop (includes #339). All affected files run green together (98 passed locally, including #339's test_fixture_generator_session.py):

tests/test_inline_generator_sessions.py ..........
tests/test_fixture_generator_session.py .........
tests/test_migration_assistant.py ..................
tests/test_test_generation.py ...........................
tests/integration/test_migration_workflow.py ....................
tests/e2e/test_qa_test_generation_flow.py ...
tests/e2e/test_qa_test_generation_integration.py ...........

(One pre-existing RuntimeWarning: coroutine 'run_migration_assistant' was never awaited remains in TestMigrationCLIIntegration — a class this PR does not touch.)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Refactor

    • Agent session execution consolidated to use a unified pattern across all agent types for improved consistency and error propagation.
  • Tests

    • Expanded test coverage for agent session behavior, including error scenarios, system prompt handling, and coverage-improvement workflows.

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fb4d7bd3-d1da-4b83-9ac4-a6e956e022db

📥 Commits

Reviewing files that changed from the base of the PR and between dd5673e and dcd7c66.

📒 Files selected for processing (9)
  • apps/backend/agents/agent_subprocess.py
  • apps/backend/agents/documentation_generator.py
  • apps/backend/agents/migration_assistant.py
  • apps/backend/agents/test_generator.py
  • tests/e2e/test_qa_test_generation_flow.py
  • tests/integration/test_migration_workflow.py
  • tests/test_inline_generator_sessions.py
  • tests/test_migration_assistant.py
  • tests/test_test_generation.py

📝 Walkthrough

Walkthrough

All four inline agent modules (agent_subprocess, documentation_generator, migration_assistant, test_generator) are refactored to use a shared agents.session.run_agent_session() helper inside async with client blocks, replacing direct client.create_agent_session() calls. A new regression test module is added, and existing test files are updated to mock the shared helper.

Changes

Agent Session API Migration

Layer / File(s) Summary
Agent implementations migrated to shared session runner
apps/backend/agents/agent_subprocess.py, apps/backend/agents/documentation_generator.py, apps/backend/agents/migration_assistant.py, apps/backend/agents/test_generator.py
All four agent modules replace client.create_agent_session() with run_agent_session() from agents.session inside async with client blocks. Each builds a combined session_message (prompt + starting message), adds explicit status == "error" early-return handling, and returns a structured result. The test_generator improvement iteration additionally treats session errors as non-fatal.
New inline session regression tests
tests/test_inline_generator_sessions.py
New test module adds _FakeSDKClient (intentionally omitting create_agent_session), _session_result() factory, and per-agent tests asserting correct run_agent_session wiring, async with lifecycle, prompt prefixing, and error status propagation for all four migrated agents.
Existing test suite updated to mock shared session runner
tests/test_migration_assistant.py, tests/test_test_generation.py, tests/integration/test_migration_workflow.py, tests/e2e/test_qa_test_generation_flow.py
All existing test files remove mock_client.create_agent_session mocking and add autouse fixtures that patch agents.session.run_agent_session with AsyncMock returning a success 4-tuple; failure simulation switches from create_agent_session.side_effect to mock_session_runner.side_effect.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant AgentModule
  participant SDKClient
  participant run_agent_session as agents.session.run_agent_session

  Caller->>AgentModule: run_*_session(starting_message, spec_dir)
  AgentModule->>AgentModule: session_message = prompt + starting_message
  AgentModule->>SDKClient: async with client (connect)
  AgentModule->>run_agent_session: await run_agent_session(client, session_message, spec_dir, phase)
  run_agent_session-->>AgentModule: (status, response_text, session_name, ...)
  SDKClient-->>AgentModule: async with exit (disconnect)
  alt status == "error"
    AgentModule-->>Caller: {success: False, error: response_text, output: None}
  else success
    AgentModule-->>Caller: {success: True, output: {response, agent_type, session_name}}
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • OBenner/Auto-Coding#68: Introduces the migration assistant agent module in apps/backend/agents/migration_assistant.py, which is one of the four files refactored in this PR to use the shared session runner.
  • OBenner/Auto-Coding#40: Modifies run_test_generator_session in apps/backend/agents/test_generator.py to extend session flow logic, directly overlapping with this PR's refactoring of the same function's session execution path.

Poem

🐰 Hopping through the code with glee,
No more phantom API calls for me!
run_agent_session — one path, shared and true,
async with client keeps the lifecycle in view.
Each agent now speaks with a unified voice,
The rabbit approves of this elegant choice! 🎉

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/sharp-edison-d7f96a

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added area/backend bug Something isn't working size/L labels Jun 13, 2026
Comment thread tests/test_inline_generator_sessions.py Outdated
…he phantom create_agent_session

ClaudeSDKClient (claude_agent_sdk) has no `create_agent_session` method, so
every inline call site raised
`'ClaudeSDKClient' object has no attribute 'create_agent_session'` at runtime.
The shared generator boilerplate (agents/_generator_base) was fixed in #339;
this covers the remaining inline call sites it called out as a follow-up:

- documentation_generator.run_documentation_generator_session
- migration_assistant.run_migration_assistant
- test_generator.run_test_generator_session (writer + coverage-improvement)
- agent_subprocess.run_agent_session (isolated-subprocess wrapper)

Each now drives the session through run_agent_session() (agents/session.py)
inside `async with client` — the same API the planner/coder/qa phases use —
and treats a returned status == "error" as failure. The agent role prompt is
delivered as the leading message since the SDK client only carries the generic
base system prompt. agent_subprocess imports the helper under an alias so it
does not shadow its own run_agent_session() wrapper.

Add tests/test_inline_generator_sessions.py (mirrors
tests/test_fixture_generator_session.py: a fake async-context-manager client
with no create_agent_session attribute, agents.session.run_agent_session
patched as an AsyncMock, asserting it is awaited and the client is entered).
Update the existing migration/test-generation suites that mocked the phantom
method to drive the shared session helper instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Oleg Miagkov <mrobenner@gmail.com>
@OBenner
OBenner force-pushed the claude/sharp-edison-d7f96a branch from 48a2f3f to 6f92d11 Compare June 13, 2026 07:11
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
@OBenner
OBenner merged commit ba9038d into develop Jun 14, 2026
13 of 14 checks passed
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/backend bug Something isn't working size/L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants