[MAJOR] §4: Synchronous getMessage() polling inside async coroutine in ChiefArchitectAgent

[mvillmow]

Finding

Severity: MAJOR
Section: 4
Evidence: src/agents/chief_architect_agent.cpp:60-66
Principle: SOLID

ChiefArchitectAgent::sendCommand() calls the synchronous getMessage() method inside a C++20 coroutine (co_return / co_await function). This creates a broken async contract: the coroutine blocks waiting for a message via synchronous polling rather than suspending. This is a latent race condition and architectural inconsistency that would be triggered under concurrent message bus usage.

---

Part of #504

[mvillmow]

Implementation Plan

Now I have enough detail to write a precise, actionable implementation plan.

---

Implementation Plan: Issue #509 — Fix Synchronous getMessage() Inside sendCommand() Coroutine

1. Objective

Replace the synchronous getMessage() call at src/agents/chief_architect_agent.cpp:59 with a properly suspending async alternative. The current code calls getMessage() (which returns immediately with nullopt if no message is queued) inside a coroutine, creating a broken async contract: if the response hasn't arrived yet, the coroutine silently returns an error instead of suspending to wait.

---

2. Approach

Key insight: getMessage() is non-blocking by design — it returns std::nullopt immediately if the inbox is empty. Calling it right after sendMessage() in a coroutine is a timing bug: the response from the downstream TaskAgent may not have arrived yet.

Chosen strategy: polling awaitable with bounded retry

Add a free function awaitMessage(AgentCore&, deadline) that returns a concurrency::Task<std::optional<core::KeystoneMessage>>. Internally it polls getMessage() in a short loop, yielding the coroutine between polls via a lightweight awaitable. This avoids introducing a full condition-variable/callback mechanism into AgentCore (which would be a large refactor) while correctly suspending rather than blocking.

This is explicitly the "interim bridging" approach recommended in the Prior Learnings. A full awaitable refactor of AgentCore is out of scope for this fix.

Rejected alternatives:

• std::condition_variable inside AgentCore — large refactor, mutex contention, requires modifying the receiveMessage path
• Busy-spin with std::this_thread::sleep_for — blocks the thread, same problem as before

---

3. Files to Create

File	Purpose
include/agents/agent_awaitable.hpp	awaitMessage() free function template + YieldAwaitable helper
tests/unit/agents/test_chief_architect_agent.cpp	Unit tests for sendCommand() async behavior

---

4. Files to Modify

File	Change
src/agents/chief_architect_agent.cpp	Replace getMessage() at line 59 with co_await awaitMessage(*this, deadline)
include/agents/chief_architect_agent.hpp	Add #include "agents/agent_awaitable.hpp"

---

5. Implementation Steps

Step 1 — Create include/agents/agent_awaitable.hpp

Add two things:

1. YieldAwaitable — a trivial awaitable that suspends once and immediately resumes, giving the scheduler a chance to run other coroutines between polls:

struct YieldAwaitable {
    bool await_ready() const noexcept { return false; }
    void await_suspend(std::coroutine_handle<> h) const noexcept {
        auto* sched = concurrency::getCurrentScheduler();
        if (sched) { sched->submit(h); }
        else { h.resume(); }  // no scheduler: resume inline
    }
    void await_resume() const noexcept {}
};

2. awaitMessage() free function:

// include/agents/agent_awaitable.hpp
concurrency::Task<std::optional<core::KeystoneMessage>>
awaitMessage(AgentCore& agent,
             std::chrono::steady_clock::time_point deadline
               = std::chrono::steady_clock::now() + std::chrono::milliseconds{500});

Implementation polls agent.getMessage() in a loop with co_await YieldAwaitable{} between retries, returning std::nullopt if the deadline passes. Default deadline: 500 ms (enough for Phase 1 test scenarios; configurable by caller).

Step 2 — Modify src/agents/chief_architect_agent.cpp lines 56-64

Replace:

// lines 56-64 (current)
// For Phase 1 synchronous testing, task agent processes immediately
// and sends response back via MessageBus
// Get the response
auto response_msg = getMessage();
if (response_msg) {
    co_return co_await processMessage(*response_msg);
}
co_return core::Response::createError(msg, agent_id_, "No response received");

With:

auto response_msg = co_await awaitMessage(*this);
if (response_msg) {
    co_return co_await processMessage(*response_msg);
}
co_return core::Response::createError(msg, agent_id_, "No response received");

Add #include "agents/agent_awaitable.hpp" to the include block at the top of the .cpp.

Step 3 — Create tests/unit/agents/test_chief_architect_agent.cpp

Tests needed:

1. SendCommand_GetsResponseFromTaskAgent — wire up two agents through a MessageBus, send a command, verify the coroutine suspends and returns the response (drives the coroutine synchronously via Task::sync_wait() or equivalent).
2. SendCommand_ReturnsErrorOnTimeout — send a command to a non-responsive agent, verify Response::isError() is true after the deadline expires.
3. SendCommand_HandlesImmediateResponse — response already in inbox before co_await, verify no spurious suspension.
4. ProcessMessage_CancelTask — exercise the existing CANCEL_TASK path (line 21-33) to avoid regression.

Step 4 — Verify no getMessage() callers remain in coroutine context

Search the codebase for any other coroutine functions that call getMessage() directly and apply the same fix if found.

---

6. Verification

# Formatting
just format-check

# Static analysis
just lint

# Build under TSan (will surface any remaining data races)
cmake --preset tsan && cmake --build --preset tsan

# Run all tests under TSan
ctest --preset tsan --output-on-failure -R "chief_architect"

# Run all tests under ASan
cmake --preset asan && cmake --build --preset asan
ctest --preset asan --output-on-failure -R "chief_architect"

# Full test suite
just test

TSan confirmation: if the race is eliminated, no DATA RACE reports should appear in the chief_architect_agent tests when run with concurrent message delivery.

---

7. Skills Used

Directly invoked during planning:

• Explore subagent — codebase research (two passes: architecture survey + specific file reads)

Team knowledge base skills referenced from Prior Learnings:

• cpp-ci-sanitizer-tsan-data-race-fixes — TSan data race detection patterns; mutex/atomic for shared state; GCC 10.2 vs clang-18 compiler requirement for C++20 coroutines
• e2e-homeric-compose-cpp-pipeline — C++20 service async patterns; lambda capture in async contexts; Keystone coroutine layer status
• batch-subprocess-signal-hang — Synchronous-blocking-in-async anti-pattern; analogous problem domain
• code-quality-audit-principles — SOLID audit framework; interface segregation relevant to keeping AgentCore minimal

---

Generated by Claude Code Planner