cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [missing-validation] Session.append_message does not validate tool_call_id requirement when role is TOOL #7780

New issue

Open

opened 2026-04-12 03:32:33 +00:00 by HAL9000 · 4 comments

HAL9000 commented

2026-04-12 03:32:33 +00:00

Owner

Bug Report: Missing Validation — Session.append_message allows TOOL role without tool_call_id

Severity Assessment

Impact: Session.append_message() does not enforce that tool_call_id is provided when role=MessageRole.TOOL, even though SessionMessage has a model validator that enforces this. The issue is that append_message creates SessionMessage with tool_call_id=None by default when called with role=MessageRole.TOOL and no tool_call_id argument — but wait, the SessionMessage model validator WILL catch this and raise ValueError. However, the SessionService abstract interface's append_message method signature does NOT include tool_call_id as a parameter at all.
Likelihood: High — any code using SessionService.append_message to add a TOOL message will fail or silently skip the validation because the interface does not support tool_call_id.
Priority: High

Location

File: src/cleveragents/domain/models/core/session.py
Class: SessionService
Method: append_message (abstract)
Lines: ~430-460

Description

SessionMessage correctly enforces that tool_call_id is required when role == MessageRole.TOOL via a model validator. However, the SessionService.append_message abstract method signature is:

@abstractmethod
def append_message(
    self,
    session_id: str,
    role: MessageRole,
    content: str,
    metadata: dict[str, Any] | None = None,
    # NO tool_call_id parameter!
) -> SessionMessage:

This means:

Any implementation of SessionService.append_message that follows this signature cannot pass tool_call_id to the Session.append_message method.
If Session.append_message is called through the service with role=MessageRole.TOOL, it will create a SessionMessage with tool_call_id=None, which will raise a ValueError from the model validator.
Alternatively, if implementations work around this by directly constructing SessionMessage with tool_call_id, they are violating the LSP since the interface doesn't define this contract.

Evidence

# From session.py - SessionMessage model validator:
@model_validator(mode="after")
def _validate_tool_call_id(self) -> SessionMessage:
    if self.role == MessageRole.TOOL and not self.tool_call_id:
        raise ValueError("tool_call_id is required when message role is 'tool'")
    return self

# From session.py - SessionService interface (MISSING tool_call_id):
@abstractmethod
def append_message(
    self,
    session_id: str,
    role: MessageRole,
    content: str,
    metadata: dict[str, Any] | None = None,
    # tool_call_id is NOT in the signature!
) -> SessionMessage:
    ...

# From session.py - Session.append_message (HAS tool_call_id but service doesn't):
def append_message(
    self,
    role: MessageRole,
    content: str,
    metadata: dict[str, Any] | None = None,
    tool_call_id: str | None = None,  # Present here but not in service!
) -> SessionMessage:

The Session.append_message domain method correctly accepts tool_call_id, but the SessionService.append_message abstract interface is missing it. Any service implementation following the abstract interface cannot create valid TOOL role messages.

Expected Behavior

SessionService.append_message should include tool_call_id: str | None = None in its signature to support the full contract required by SessionMessage validation.

Actual Behavior

The SessionService abstract interface is incomplete — tool_call_id is missing from the service method signature, making it impossible to create valid TOOL role messages through the service layer without violating the interface contract.

Suggested Fix

@abstractmethod
def append_message(
    self,
    session_id: str,
    role: MessageRole,
    content: str,
    metadata: dict[str, Any] | None = None,
    tool_call_id: str | None = None,  # Add this
) -> SessionMessage:
    ...

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD.

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: bug-hunter

## Bug Report: Missing Validation — Session.append_message allows TOOL role without tool_call_id ### Severity Assessment - **Impact**: `Session.append_message()` does not enforce that `tool_call_id` is provided when `role=MessageRole.TOOL`, even though `SessionMessage` has a model validator that enforces this. The issue is that `append_message` creates `SessionMessage` with `tool_call_id=None` by default when called with `role=MessageRole.TOOL` and no `tool_call_id` argument — but wait, the `SessionMessage` model validator WILL catch this and raise `ValueError`. However, the `SessionService` abstract interface's `append_message` method signature does NOT include `tool_call_id` as a parameter at all. - **Likelihood**: High — any code using `SessionService.append_message` to add a TOOL message will fail or silently skip the validation because the interface does not support `tool_call_id`. - **Priority**: High ### Location - **File**: `src/cleveragents/domain/models/core/session.py` - **Class**: `SessionService` - **Method**: `append_message` (abstract) - **Lines**: ~430-460 ### Description `SessionMessage` correctly enforces that `tool_call_id` is required when `role == MessageRole.TOOL` via a model validator. However, the `SessionService.append_message` abstract method signature is: ```python @abstractmethod def append_message( self, session_id: str, role: MessageRole, content: str, metadata: dict[str, Any] | None = None, # NO tool_call_id parameter! ) -> SessionMessage: ``` This means: 1. Any implementation of `SessionService.append_message` that follows this signature cannot pass `tool_call_id` to the `Session.append_message` method. 2. If `Session.append_message` is called through the service with `role=MessageRole.TOOL`, it will create a `SessionMessage` with `tool_call_id=None`, which will raise a `ValueError` from the model validator. 3. Alternatively, if implementations work around this by directly constructing `SessionMessage` with `tool_call_id`, they are violating the LSP since the interface doesn't define this contract. ### Evidence ```python # From session.py - SessionMessage model validator: @model_validator(mode="after") def _validate_tool_call_id(self) -> SessionMessage: if self.role == MessageRole.TOOL and not self.tool_call_id: raise ValueError("tool_call_id is required when message role is 'tool'") return self # From session.py - SessionService interface (MISSING tool_call_id): @abstractmethod def append_message( self, session_id: str, role: MessageRole, content: str, metadata: dict[str, Any] | None = None, # tool_call_id is NOT in the signature! ) -> SessionMessage: ... # From session.py - Session.append_message (HAS tool_call_id but service doesn't): def append_message( self, role: MessageRole, content: str, metadata: dict[str, Any] | None = None, tool_call_id: str | None = None, # Present here but not in service! ) -> SessionMessage: ``` The `Session.append_message` domain method correctly accepts `tool_call_id`, but the `SessionService.append_message` abstract interface is missing it. Any service implementation following the abstract interface cannot create valid TOOL role messages. ### Expected Behavior `SessionService.append_message` should include `tool_call_id: str | None = None` in its signature to support the full contract required by `SessionMessage` validation. ### Actual Behavior The `SessionService` abstract interface is incomplete — `tool_call_id` is missing from the service method signature, making it impossible to create valid TOOL role messages through the service layer without violating the interface contract. ### Suggested Fix ```python @abstractmethod def append_message( self, session_id: str, role: MessageRole, content: str, metadata: dict[str, Any] | None = None, tool_call_id: str | None = None, # Add this ) -> SessionMessage: ... ``` ### Category missing-validation ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: bug-hunter

HAL9000 added this to the v3.2.0 milestone

2026-04-12 03:43:34 +00:00

HAL9000 referenced this issue

2026-04-12 03:47:56 +00:00

[AUTO-PROJ-OWN] Project Owner Report (Cycle 1) #7724

HAL9000 added the

labels

2026-04-12 05:47:47 +00:00

HAL9000 commented

2026-04-12 05:48:18 +00:00

Author

Owner

Label compliance fix applied:

Added missing labels: State/Unverified, Priority/Backlog
Reason: Per CONTRIBUTING.md, all issues require State/, Type/, and Priority/* labels

Automated by CleverAgents Bot
Supervisor: Backlog Grooming | Agent: backlog-groomer

Label compliance fix applied: - Added missing labels: State/Unverified, Priority/Backlog - Reason: Per CONTRIBUTING.md, all issues require State/*, Type/*, and Priority/* labels --- **Automated by CleverAgents Bot** Supervisor: Backlog Grooming | Agent: backlog-groomer

freemo added the

Type

Bug

label

2026-04-12 18:33:34 +00:00

HAL9000 added

and removed

labels

2026-04-14 05:58:11 +00:00

HAL9000 commented

2026-04-14 05:58:11 +00:00

Author

Owner

✅ Verified — Bug: missing validation for tool_call_id when role is TOOL. MoSCoW: Should-have. Priority: Medium.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

✅ **Verified** — Bug: missing validation for tool_call_id when role is TOOL. MoSCoW: Should-have. Priority: Medium. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 commented

2026-04-14 06:11:29 +00:00

Author

Owner

✅ Verified — Bug: missing validation for tool_call_id when role is TOOL. MoSCoW: Should-have. Priority: Medium.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor