fix(acp): await task/create instead of backgrounding it

[chakrris]

Summary

task/create was dispatched in the background, so the ACP server handed back a task id before its handler had started the Temporal workflow. A caller that followed task/create with event/send could have its signal arrive first and be dropped with workflow not found, while seeing success on both calls. This makes task/create synchronous and leaves event/send alone.

What changed

src/agentex/protocol/acp.py — RPC_SYNC_METHODS gains TASK_CREATE.

Only MESSAGE_SEND was in that list, so everything else took the background branch in BaseACPServer._handle_jsonrpc:

if method in RPC_SYNC_METHODS:
    result = await handler(params)
    ...
else:
    asyncio.create_task(self._process_request(rpc_request.id, method, params))
    return JSONRPCResponse(id=rpc_request.id, result={"status": "processing"})

task/create landed there by default rather than by decision. Its contract is to return an id for a task that now exists, and TemporalACP.handle_task_create is what runs start_workflow, so answering early returns an id the server cannot yet route to. Nothing batches task creations, so there is no benefit being traded away, and start_workflow is a short RPC.

tests/test_acp_sync_dispatch.py — new, and pins both directions so neither is changed by accident:

• task/create completes its handler before responding, and a raising handler yields a JSON-RPC error rather than a success.
• event/send still acknowledges with {"status": "processing"} before its handler runs.

The task/create handlers yield with await asyncio.sleep(0) first, so a regression to background dispatch loses the ordering race and fails.

Why event/send stays asynchronous

An earlier revision of this PR made event/send synchronous too, and it broke batching. The 10_async/00_base/080_batch_events tutorial sends events in quick succession and asserts the workflow drains more than one per batch:

assert found_batch_with_multiple_events, "Should have found a batch with multiple events"
E   AssertionError: assert False

Awaiting each send serialises them, so no batch ever holds more than one event. Two of that tutorial's tests failed, reproducibly. Background dispatch there is deliberate design.

Once task/create is synchronous the workflow is addressable before any event is sent, which is what the race actually needed. The two calls have different contracts: event/send is a legitimate fire-and-forget, task/create is a creation call.

Review guide

Start with src/agentex/protocol/acp.py; the change is one line and a comment. The behaviour it selects is in src/agentex/lib/sdk/fastacp/base/base_acp_server.py around _handle_jsonrpc and _process_request, unchanged here but worth reading alongside. The handlers are in src/agentex/lib/sdk/fastacp/impl/temporal_acp.py.

Compatibility

The synchronous path returns the handler's return value, and handle_task_create returns None. The agentex backend reads this as return rpc_response.result or {} and raises only on rpc_response.error, so a None result is unchanged behaviour for it. Its HTTP call to the ACP already carries a 60 s timeout, which bounds the new blocking.

Not fixed here

A send to a genuinely dead target — a completed or terminated workflow — still fails silently. handle_event_send raises, but the raise lands in _process_request, which only logs, and RPCMethod has no status method for a caller to ask after the fact. Reporting that without serialising sends needs its own design, so it is left for separate work.

How this was found

An eval harness lost work to it repeatedly: workflows started, never received their payload, and sat on their wait timer until the client's timeout fired. Sixteen occurrences in one day across two agents, each costing a lost item and the full client timeout.

Testing

I could not run the suite locally: uv here is 0.8.13 against the repo's required-version >=0.9, and this machine's pip index needs credentials I do not have. The new tests need neither Temporal nor network, so CI covers them.

Confidence Score: 4/5

This PR is not safe to merge because EVENT_SEND can lose an event while telling the caller it succeeded.

Findings

1.  Event failures look successful ▶

Fix with agent prompt

### Issue 1
src/agentex/protocol/acp.py:139-143
`EVENT_SEND` still returns `{"status": "processing"}` before `send_signal` runs. If Temporal rejects the signal, `_process_request` only logs the error. Await `EVENT_SEND` so the caller gets a JSON-RPC error and can retry. Separate HTTP requests can still run at the same time, so waiting does not prevent callers from sending events quickly.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

ACP task creation now waits for its Temporal workflow to start before replying, so a following event can reach an existing workflow. Event sending stays asynchronous so callers can continue sending events without waiting for each one.

• Makes task creation synchronous.
• Keeps event delivery asynchronous.
• Adds tests for ordering, errors, and acknowledgments.

Diagram

sequenceDiagram
    participant C as Caller
    participant A as ACP server
    participant T as Temporal
    C->>A: task/create
    A->>T: start_workflow
    T-->>A: workflow started
    A-->>C: JSON-RPC result
    C->>A: event/send
    A-->>C: "{status: processing}"
    A->>T: send_signal in background
    alt signal succeeds
        T-->>A: accepted
    else signal fails
        T-->>A: error
        A->>A: log error only
    end

Loading

_{Reviews (4) · Last reviewed commit: "fix(acp): keep event/send asynchronous, ..."}

[greptile-apps]

EVENT_SEND still returns {"status": "processing"} before send_signal runs. If Temporal rejects the signal, _process_request only logs the error. Await EVENT_SEND so the caller gets a JSON-RPC error and can retry. Separate HTTP requests can still run at the same time, so waiting does not prevent callers from sending events quickly.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/agentex/protocol/acp.py
Line: 139-143

Comment:
`EVENT_SEND` still returns `{"status": "processing"}` before `send_signal` runs. If Temporal rejects the signal, `_process_request` only logs the error. Await `EVENT_SEND` so the caller gets a JSON-RPC error and can retry. Separate HTTP requests can still run at the same time, so waiting does not prevent callers from sending events quickly.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.