fix(standalone): repair in-process execution flow#780
Merged
qdaxb merged 5 commits intoApr 10, 2026
Conversation
Contributor
📝 WalkthroughWalkthroughInprocessExecutor now builds agents from an ExecutionRequest, awaits async pre_execute (failing flows return FAILED), removes the synchronous post_execute step, and raises RuntimeError when the agent lifecycle returns FAILED. Unit tests were added to validate success and failure paths. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Executor as InprocessExecutor
participant AgentService
participant Agent
participant Emitter as TaskEmitter
Caller->>Executor: execute(request)
Executor->>AgentService: create_agent(request)
AgentService-->>Executor: agent
Executor->>Agent: _execute_agent_async(agent, request)
Agent->>Agent: pre_execute() (await)
alt pre_execute SUCCESS
Agent->>Agent: execute_async() (await)
alt execute COMPLETED
Agent->>Emitter: emit_complete()
Emitter-->>Executor: ack
Executor-->>Caller: return success
else execute FAILED
Agent->>Emitter: emit_error(error_message)
Emitter-->>Executor: ack
Executor->>Caller: raise RuntimeError(error_message)
end
else pre_execute FAILED
Agent->>Emitter: emit_error(pre_error + lifecycle info)
Emitter-->>Executor: ack
Executor->>Caller: raise RuntimeError(combined_error)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
backend/app/services/execution/inprocess_executor.py (2)
399-428:⚠️ Potential issue | 🟠 MajorRestore
post_execute()to keep lifecycle parity withAgent.handle().
_execute_agent_async()now returns right after the execute phase, so standalone mode skips any agent teardown/finalization implemented inpost_execute(). That diverges from the normal agent lifecycle and can leave cleanup/persistence hooks unused. The helper-level test that currently bakes in “skips missing post_execute” will need updating once this is restored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/execution/inprocess_executor.py` around lines 399 - 428, The code returns immediately after the execute phase and never invokes the agent.post_execute() lifecycle method, so restore lifecycle parity by calling agent.post_execute() after the execute result (whether success or failure) and returning its result or preserving execute's status plus any post_execute error; specifically update the async helper that calls agent.pre_execute(), agent.execute_async()/agent.execute() (the block using asyncio.get_running_loop() and loop.run_in_executor) to always call agent.post_execute() (handle both sync/async post_execute if present) and include any post_execute error in the returned tuple instead of returning right after execute.
352-364:⚠️ Potential issue | 🟠 MajorAlways clean up task clients on failed executions.
A failed run is still task completion. Right now any
TaskStatus.FAILEDor exception path jumps past_cleanup_task_clients(), so standalone failures can leave task-scoped Claude clients/sessions alive. Move cleanup into afinallyblock; the failure-path regression test should flip with that change.♻️ Suggested change
- # Cleanup: Close Claude Code client after task completion - # This releases resources since standalone mode doesn't need to keep - # clients alive for container reuse like Docker mode does - await self._cleanup_task_clients(request.task_id) - except Exception as e: logger.exception( f"[InprocessExecutor] Execution error: " f"task_id={request.task_id}, error={e}" ) # Emit error event await emitter.emit_error( task_id=request.task_id, subtask_id=request.subtask_id, error=str(e), message_id=request.message_id, ) raise + finally: + # Cleanup: Close Claude Code client after task completion + # This releases resources since standalone mode doesn't need to keep + # clients alive for container reuse like Docker mode does + await self._cleanup_task_clients(request.task_id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/execution/inprocess_executor.py` around lines 352 - 364, The cleanup call self._cleanup_task_clients(request.task_id) must run regardless of success or failure: wrap the execution/result handling so that any raised RuntimeError or other exceptions (including the TaskStatus.FAILED branch that currently raises) do not bypass cleanup by moving the await self._cleanup_task_clients(request.task_id) into a finally block; ensure the finally executes after handling the status/raising the error so the FAILED path still raises but still calls _cleanup_task_clients for the given request.task_id.
🧹 Nitpick comments (2)
backend/tests/services/execution/test_inprocess_executor.py (2)
13-15: Annotate the new async tests with-> None.These are all new Python functions, so they should carry explicit return annotations to match the repo typing rule.
As per coding guidelines, "Python: Follow PEP 8, Black formatter (line length: 88), isort, type hints required".✍️ Suggested change
-async def test_execute_passes_execution_request_to_agent_service(): +async def test_execute_passes_execution_request_to_agent_service() -> None: @@ -async def test_execute_agent_async_awaits_pre_execute_and_skips_missing_post_execute(): +async def test_execute_agent_async_awaits_pre_execute_and_skips_missing_post_execute() -> None: @@ -async def test_execute_raises_and_emits_error_when_agent_lifecycle_returns_failed(): +async def test_execute_raises_and_emits_error_when_agent_lifecycle_returns_failed() -> None:Also applies to: 48-50, 66-68
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/services/execution/test_inprocess_executor.py` around lines 13 - 15, Add explicit return type annotations "-> None" to the new async test functions (e.g., test_execute_passes_execution_request_to_agent_service and the other tests referenced at lines 48-50 and 66-68) so they follow the repository typing rule; update each async def signature to include "-> None" (for example: async def test_execute_passes_execution_request_to_agent_service(...) -> None:) while leaving the function body unchanged.
48-64: Add the matching_execute_agent_async()failure-path test.The helper-level coverage stops at the happy path. The new branch that awaits
pre_execute()and foldspre_errorinto the returned message is still untested because the outer failure test stubs_execute_agent_async()itself.🧪 Suggested test
`@pytest.mark.asyncio` async def test_execute_agent_async_awaits_pre_execute_and_skips_missing_post_execute(): executor = InprocessExecutor() agent = SimpleNamespace( task_id=3, @@ assert error is None agent.pre_execute.assert_awaited_once() agent.execute_async.assert_awaited_once() + +@pytest.mark.asyncio +async def test_execute_agent_async_returns_failed_when_pre_execute_fails() -> None: + executor = InprocessExecutor() + agent = SimpleNamespace( + task_id=3, + get_name=lambda: "fake-agent", + pre_execute=AsyncMock(return_value=(TaskStatus.FAILED, "boom")), + execute_async=AsyncMock(), + ) + + status, error = await executor._execute_agent_async(agent) + + assert status == TaskStatus.FAILED + assert "Pre-execute failed" in error + assert "boom" in error + agent.execute_async.assert_not_awaited()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/services/execution/test_inprocess_executor.py` around lines 48 - 64, The test suite lacks coverage for the failure path inside InprocessExecutor._execute_agent_async where agent.pre_execute() returns a failure and its pre_error should be folded into the returned error; add a new async test (e.g., test_execute_agent_async_handles_pre_execute_failure) that constructs an agent with pre_execute=AsyncMock(return_value=(TaskStatus.FAILED, "pre_error")), execute_async=AsyncMock(), then awaits executor._execute_agent_async(agent) and asserts the returned status equals TaskStatus.FAILED, the returned error equals "pre_error" (or contains it), and that agent.pre_execute.assert_awaited_once() is called while agent.execute_async.assert_not_awaited() is used to ensure execute_async is skipped.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@backend/app/services/execution/inprocess_executor.py`:
- Around line 399-428: The code returns immediately after the execute phase and
never invokes the agent.post_execute() lifecycle method, so restore lifecycle
parity by calling agent.post_execute() after the execute result (whether success
or failure) and returning its result or preserving execute's status plus any
post_execute error; specifically update the async helper that calls
agent.pre_execute(), agent.execute_async()/agent.execute() (the block using
asyncio.get_running_loop() and loop.run_in_executor) to always call
agent.post_execute() (handle both sync/async post_execute if present) and
include any post_execute error in the returned tuple instead of returning right
after execute.
- Around line 352-364: The cleanup call
self._cleanup_task_clients(request.task_id) must run regardless of success or
failure: wrap the execution/result handling so that any raised RuntimeError or
other exceptions (including the TaskStatus.FAILED branch that currently raises)
do not bypass cleanup by moving the await
self._cleanup_task_clients(request.task_id) into a finally block; ensure the
finally executes after handling the status/raising the error so the FAILED path
still raises but still calls _cleanup_task_clients for the given
request.task_id.
---
Nitpick comments:
In `@backend/tests/services/execution/test_inprocess_executor.py`:
- Around line 13-15: Add explicit return type annotations "-> None" to the new
async test functions (e.g.,
test_execute_passes_execution_request_to_agent_service and the other tests
referenced at lines 48-50 and 66-68) so they follow the repository typing rule;
update each async def signature to include "-> None" (for example: async def
test_execute_passes_execution_request_to_agent_service(...) -> None:) while
leaving the function body unchanged.
- Around line 48-64: The test suite lacks coverage for the failure path inside
InprocessExecutor._execute_agent_async where agent.pre_execute() returns a
failure and its pre_error should be folded into the returned error; add a new
async test (e.g., test_execute_agent_async_handles_pre_execute_failure) that
constructs an agent with pre_execute=AsyncMock(return_value=(TaskStatus.FAILED,
"pre_error")), execute_async=AsyncMock(), then awaits
executor._execute_agent_async(agent) and asserts the returned status equals
TaskStatus.FAILED, the returned error equals "pre_error" (or contains it), and
that agent.pre_execute.assert_awaited_once() is called while
agent.execute_async.assert_not_awaited() is used to ensure execute_async is
skipped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ba337d7-d06f-49a9-b59e-a039b77a5557
📒 Files selected for processing (2)
backend/app/services/execution/inprocess_executor.pybackend/tests/services/execution/test_inprocess_executor.py
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
backend/app/services/execution/inprocess_executor.py (2)
404-404: Redundant import:TaskStatusis already imported at module level.Line 30 already imports
TaskStatusfromshared.status. This inner import is unnecessary.🧹 Remove redundant import
- from shared.status import TaskStatus - try: # Mirror Agent.handle() semantics in the current event loop.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/execution/inprocess_executor.py` at line 404, Remove the redundant inner import of TaskStatus in inprocess_executor.py: since TaskStatus is already imported at module level (from shared.status), delete the local "from shared.status import TaskStatus" statement inside the function/block to avoid shadowing/redundant imports and keep only the top-level import.
386-389: Consider a more specific return type annotation.The return type
-> tupleis overly generic. A precise annotation improves IDE support and static analysis.✏️ Suggested type hint improvement
+from typing import Optional, Tuple + async def _execute_agent_async( self, agent, -) -> tuple: +) -> Tuple[TaskStatus, Optional[str]]:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/execution/inprocess_executor.py` around lines 386 - 389, The return type on _execute_agent_async is too broad; update the signature of async def _execute_agent_async(self, agent) -> tuple to a precise typing (e.g., Tuple[AgentExecutionResult, dict[str, Any]] or Tuple[AgentExecutionResult, ExecutionMetadata]) that matches the actual values returned by the function, add the required imports from typing (Tuple, Optional, Any) or reference the concrete classes (AgentExecutionResult, ExecutionMetadata) used elsewhere, and adjust any callers/tests if needed to use the more specific types to improve static analysis and IDE completions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/app/services/execution/inprocess_executor.py`:
- Line 404: Remove the redundant inner import of TaskStatus in
inprocess_executor.py: since TaskStatus is already imported at module level
(from shared.status), delete the local "from shared.status import TaskStatus"
statement inside the function/block to avoid shadowing/redundant imports and
keep only the top-level import.
- Around line 386-389: The return type on _execute_agent_async is too broad;
update the signature of async def _execute_agent_async(self, agent) -> tuple to
a precise typing (e.g., Tuple[AgentExecutionResult, dict[str, Any]] or
Tuple[AgentExecutionResult, ExecutionMetadata]) that matches the actual values
returned by the function, add the required imports from typing (Tuple, Optional,
Any) or reference the concrete classes (AgentExecutionResult, ExecutionMetadata)
used elsewhere, and adjust any callers/tests if needed to use the more specific
types to improve static analysis and IDE completions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 00ed9d49-1e84-497e-86c6-705ca5fefb6b
📒 Files selected for processing (2)
backend/app/services/execution/inprocess_executor.pybackend/tests/services/execution/test_inprocess_executor.py
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/tests/services/execution/test_inprocess_executor.py (1)
89-121: Consider adding test coverage for exception handling and sync fallback paths.The current tests cover the main success and failure paths well. For more comprehensive coverage, consider adding tests for:
Exception during execution: When
execute_async()raises an exception,_execute_agent_asynccatches it and returns(TaskStatus.FAILED, str(e))per context snippet 1 lines 434-438.Sync execute fallback: When an agent lacks
execute_async, the implementation falls back to runningagent.execute()in an executor (context snippet 1 lines 424-428).These would strengthen regression coverage for edge cases.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/tests/services/execution/test_inprocess_executor.py` around lines 89 - 121, Add two new tests: one that verifies _execute_agent_async maps exceptions into (TaskStatus.FAILED, str(e)) by patching the agent's execute_async to raise and asserting execute returns/propagates the FAILED tuple and emitter emits error; and another that verifies the sync fallback path by creating an agent without execute_async (no attribute) but with a blocking execute method, then patching InprocessExecutor to run that method in a thread pool (or assert run_in_executor was called) and confirming the returned status/message is handled correctly. Target symbols: _execute_agent_async, execute_async, execute, and InprocessExecutor.execute to locate relevant code to patch/mimic. Ensure emitter.assert_awaited calls and cleanup behavior mirror existing tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/tests/services/execution/test_inprocess_executor.py`:
- Around line 89-121: Add two new tests: one that verifies _execute_agent_async
maps exceptions into (TaskStatus.FAILED, str(e)) by patching the agent's
execute_async to raise and asserting execute returns/propagates the FAILED tuple
and emitter emits error; and another that verifies the sync fallback path by
creating an agent without execute_async (no attribute) but with a blocking
execute method, then patching InprocessExecutor to run that method in a thread
pool (or assert run_in_executor was called) and confirming the returned
status/message is handled correctly. Target symbols: _execute_agent_async,
execute_async, execute, and InprocessExecutor.execute to locate relevant code to
patch/mimic. Ensure emitter.assert_awaited calls and cleanup behavior mirror
existing tests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 73885db0-abd6-4b64-b965-c3f257a45e3f
📒 Files selected for processing (1)
backend/tests/services/execution/test_inprocess_executor.py
qdaxb
approved these changes
Apr 10, 2026
cocowh
pushed a commit
to cocowh/Wegent
that referenced
this pull request
May 11, 2026
…i#780) Update message timestamp format from time-only (HH:mm:ss) to include date (YYYY-MM-DD HH:mm:ss) in chat interface, export PDF, and share export modal. Create unified formatDateTime utility function to ensure consistent date-time formatting across all components. Co-authored-by: joyway1978 <184585080+joyway1978@users.noreply.github.com>
cocowh
pushed a commit
to cocowh/Wegent
that referenced
this pull request
May 11, 2026
feat(frontend): add date display to chat message timestamps (wecode-ai#780) See merge request weibo_rd/common/wecode/wegent!994
cocowh
pushed a commit
to cocowh/Wegent
that referenced
this pull request
May 11, 2026
feat(frontend): add date display to chat message timestamps (wecode-ai#780) See merge request weibo_rd/common/wecode/wegent!1006
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ExecutionRequestdirectly toAgentServicein standalone in-process executionpre_execute()and align in-process lifecycle behavior with agent contractsTest Plan
uv run --project backend pytest backend/tests/services/execution/test_inprocess_executor.py -quv run --project backend pytest backend/tests/services/execution -qNotes
Summary by CodeRabbit
Bug Fixes
Tests