[MAJOR] §14: KeystoneMessage struct carries orchestration concerns in transport layer

[mvillmow]

Finding

Severity: MAJOR
Section: 14
Evidence: include/core/message.hpp:134 (task_id), :127 (session_id), :51 (CANCEL_TASK ActionType)
Principle: SOLID

KeystoneMessage contains task_id (for task tracking/cancellation), session_id (for context isolation), CANCEL_TASK and TASK_FAILED action types, and a metadata map. These are orchestration concerns. For a library documented as a 'pure transport layer', the message protocol is contaminated with application-level semantics that belong in ProjectAgamemnon.

---

Part of #504

[mvillmow]

Implementation Plan

Now I have all the context needed to write a comprehensive implementation plan.

---

Implementation Plan: Issue #515 — Remove Orchestration Concerns from KeystoneMessage

1. Objective

Strip orchestration-level semantics (task_id, session_id, CANCEL_TASK, TASK_FAILED, and the metadata map) from KeystoneMessage in include/core/message.hpp. These fields/values belong to ProjectAgamemnon, not to Keystone's pure transport layer. The result should be a message struct that carries only routing and payload fields, with cancellation/failure signaling moved to a higher-level protocol living in the agent layer.

---

2. Approach

Key architectural decision: Rather than deleting these concepts entirely, we relocate them upward:

• KeystoneMessage becomes a minimal transport envelope (routing headers + opaque payload).
• ActionType is reduced to transport-relevant variants only; orchestration action types (CANCEL_TASK, TASK_FAILED) move to an AgentActionType enum owned by the agent layer (include/agents/).
• session_id and task_id move into the payload (encoded in the command/payload fields as structured data, or as a new AgentMessage wrapper that sits above KeystoneMessage).
• The metadata map is removed from KeystoneMessage; if agents need key-value extensibility, they encode it in the payload.
• Cancellation tracking (requestCancellation, isCancelled, clearCancellation in AgentCore) is unaffected — it's internal agent state, not a transport concern.

This is a refactor-in-place approach: the transport tests remain unchanged; the agent-layer tests are updated to use the new AgentActionType.

What stays in KeystoneMessage:

• msg_id, sender_id, receiver_id (routing)
• priority, deadline (transport QoS)
• content_type (payload encoding hint)
• command, payload, timestamp (data)
• correlation_id (cross-host tracing — transport-level concern, stays)
• ActionType reduced to: EXECUTE, RETURN_RESULT, SHUTDOWN (all pure transport signals)

What moves out of KeystoneMessage:

• ActionType::CANCEL_TASK → agents::AgentActionType::CANCEL_TASK
• ActionType::TASK_FAILED → agents::AgentActionType::TASK_FAILED
• ActionType::DECOMPOSE → agents::AgentActionType::DECOMPOSE
• session_id → encoded in payload / AgentEnvelope wrapper
• task_id → encoded in payload / AgentEnvelope wrapper
• metadata map → removed (encode in payload if needed)

AgentEnvelope: A new thin struct in the agent layer that wraps a KeystoneMessage and adds agent-level fields. The transport layer never sees this type.

---

3. Files to Create

File	Purpose
include/agents/agent_action_type.hpp	AgentActionType enum (DECOMPOSE, CANCEL_TASK, TASK_FAILED) + agentActionTypeToString()
include/agents/agent_envelope.hpp	AgentEnvelope struct wrapping KeystoneMessage + session_id, task_id, metadata, agent_action
src/agents/agent_envelope.cpp	Factory methods AgentEnvelope::create(), AgentEnvelope::createCancellation(), AgentEnvelope::createFailure()
tests/unit/test_agent_envelope.cpp	Unit tests for AgentEnvelope construction, factory methods, round-trips

---

4. Files to Modify

include/core/message.hpp (primary target)

• Remove ActionType::DECOMPOSE, CANCEL_TASK, TASK_FAILED from ActionType enum (keep EXECUTE, RETURN_RESULT, SHUTDOWN).
• Remove std::string session_id field.
• Remove std::optional<std::string> task_id field.
• Remove std::map<std::string, std::string> metadata field.
• Remove static KeystoneMessage createCancellation(...) factory method.
• Update actionTypeToString() switch to remove deleted cases.
• Update struct docstring to remove "Phase A" and "Phase 1.2" references.

src/core/message.cpp

• Remove createCancellation() implementation (lines 104–120).
• Remove session_id initialization from both create() overloads.
• Remove metadata initialization.
• Remove the create(sender, receiver, ActionType, session, ...) overload that takes session — or keep it with the session parameter dropped.

include/core/message_serializer.hpp + src/core/message_serializer.cpp

• Remove session_id from SerializableMessage.
• Remove metadata from SerializableMessage.
• Update fromKeystoneMessage() and toKeystoneMessage() to not include these fields.

include/agents/async_agent.hpp + src/agents/async_agent.cpp

• Change handleCancellation(const core::KeystoneMessage&) signature to handleCancellation(const AgentEnvelope&).
• Update implementation to read task_id from AgentEnvelope.

include/agents/lead_agent_base.hpp + src/agents/lead_agent_base_impl.hpp

• Change processMessage() to accept AgentEnvelope or unwrap to check agent_action for CANCEL_TASK/TASK_FAILED.
• Update processSubordinateFailure() to use AgentEnvelope.
• Update isSubordinateResult() to check AgentActionType not core::ActionType.

include/agents/module_lead_agent.hpp / src/agents/module_lead_agent.cpp

• isSubordinateResult(): change check from msg.action_type != core::ActionType::TASK_FAILED → envelope.agent_action != AgentActionType::TASK_FAILED.

include/agents/component_lead_agent.hpp / src/agents/component_lead_agent.cpp

• Same pattern as module_lead_agent.

src/agents/task_agent.cpp

• Replace if (msg.action_type == core::ActionType::CANCEL_TASK) with envelope check.
• Replace explicit failure_msg.action_type = core::ActionType::TASK_FAILED with AgentEnvelope::createFailure(...).
• Remove session_id usage (lines 66, 114 in message.cpp).

src/agents/chief_architect_agent.cpp

• Update CANCEL_TASK check (line 22) to use AgentActionType.

include/agents/coordination_state.hpp

• If current_task_id is stored here for tracking: no change needed — this is internal agent state, not a transport field.

tests/unit/test_task_cancellation.cpp

• Update all msg.task_id, msg.session_id, msg.action_type == ActionType::CANCEL_TASK references to use AgentEnvelope.
• Update KeystoneMessage::createCancellation(...) calls to AgentEnvelope::createCancellation(...).

tests/unit/test_message_serializer.cpp

• Remove tests referencing session_id or metadata in KeystoneMessage.
• Add/update tests confirming these fields are NOT in SerializableMessage.

proto/common.proto (optional, low risk)

• No change required unless ActionType is directly mapped; the proto TaskActionType does not include CANCEL_TASK/TASK_FAILED already, so it's already clean.

---

5. Implementation Order

1. Create include/agents/agent_action_type.hpp — define AgentActionType enum and agentActionTypeToString(). No dependencies.

2. Create include/agents/agent_envelope.hpp and src/agents/agent_envelope.cpp — define AgentEnvelope wrapping KeystoneMessage with session_id, task_id, metadata, agent_action. Add factory methods: create(), createCancellation(), createFailure().

3. Modify include/core/message.hpp and src/core/message.cpp — strip session_id, task_id, metadata, DECOMPOSE/CANCEL_TASK/TASK_FAILED from ActionType, remove createCancellation(). This will cause compile errors in agent files — that's expected.

4. Modify include/core/message_serializer.hpp and src/core/message_serializer.cpp — remove session_id and metadata from SerializableMessage.

5. Fix src/agents/async_agent.cpp — update handleCancellation() to accept AgentEnvelope.

6. Fix src/agents/lead_agent_base_impl.hpp — update processMessage() to unpack AgentEnvelope from transport and dispatch on AgentActionType.

7. Fix src/agents/module_lead_agent.cpp and src/agents/component_lead_agent.cpp — update isSubordinateResult().

8. Fix src/agents/task_agent.cpp — update processMessage() to use AgentEnvelope.

9. Fix src/agents/chief_architect_agent.cpp — update CANCEL_TASK dispatch.

10. Update tests/unit/test_task_cancellation.cpp — migrate from KeystoneMessage factory to AgentEnvelope factory.

11. Update tests/unit/test_message_serializer.cpp — remove session_id/metadata tests, confirm absence.

12. Create tests/unit/test_agent_envelope.cpp — new unit tests for AgentEnvelope.

13. Build and verify — confirm all tests pass under asan and tsan presets.

---

6. Verification

# Step-by-step compile gating:
cmake --preset debug && cmake --build --preset debug

# Full sanitizer suites:
cmake --preset asan && cmake --build --preset asan && ctest --preset asan --output-on-failure
cmake --preset tsan && cmake --build --preset tsan && ctest --preset tsan --output-on-failure

# Formatting gate:
just format-check

# Static analysis:
just lint

# Specific test targets to confirm no regressions:
ctest --preset asan -R "TaskCancellation|MessageSerializer|AgentEnvelope" --output-on-failure

Expected outcomes:

• KeystoneMessage no longer contains session_id, task_id, metadata, or orchestration-specific ActionType values.
• SerializableMessage no longer serializes these fields.
• All existing agent behavior (cancellation, failure propagation) continues working via AgentEnvelope.
• Zero new test failures; test_agent_envelope.cpp passes all new cases.
• just lint and just format-check pass clean.

---

7. Skills Used

• hephaestus:repo-analyze-quick — not invoked directly, but the issue references the same audit framework (#504).
• Explore subagent — used to map all references to task_id, session_id, CANCEL_TASK, TASK_FAILED, and ActionType across the codebase before designing the plan.

---

Risk note: Steps 3–9 form a single atomic refactor — the codebase will not compile between steps 3 and 9. Plan to land all changes in one branch without intermediate green builds, or use a stub using session_id_removed = void; trick to catch all usages via compiler errors rather than grep.

---

Generated by Claude Code Planner