BUG-HUNT: [error-handling] Missing argument validation in file_tools.py violates project fail-fast principles #7223

Open
opened 2026-04-10 09:20:30 +00:00 by HAL9000 · 4 comments
Owner

Background and Context

The file tool handlers in src/cleveragents/tool/builtins/file_tools.py do not validate required arguments at function entry, violating the project's mandatory "fail-fast" error handling standards. Per CONTRIBUTING.md, all public and protected class methods must validate arguments first — before any other logic — covering value range, null checks, type verification, empty strings, and empty collections.

Current Behavior

Handler functions access required keys directly from the inputs dict without first verifying their existence, type, or value range:

def _handle_file_read(inputs: dict[str, Any]) -> dict[str, Any]:
    """Read file content."""
    path = validate_path(inputs["path"], inputs.get("sandbox_root"))  # ← No validation that "path" exists
    encoding: str = inputs.get("encoding", "utf-8")
    offset: int = inputs.get("offset", 0)  # ← No validation that offset is non-negative
    limit: int | None = inputs.get("limit")  # ← No validation that limit is positive

def _handle_file_write(inputs: dict[str, Any]) -> dict[str, Any]:
    """Write content to file (create or overwrite)."""
    path = validate_path(inputs["path"], inputs.get("sandbox_root"))  # ← No validation
    content: str = inputs["content"]  # ← No validation that "content" exists or is string

Specific missing validations:

  1. Missing required field checks — Functions access inputs["path"] and inputs["content"] without checking if the keys exist, causing opaque KeyError on missing input.
  2. No type validation — No verification that values are the expected types (str, int, etc.), causing TypeError or silent misbehaviour.
  3. No range validationoffset can be negative; limit can be zero or negative.
  4. No null/empty checks — Fields can be None or empty strings without validation.

Affected functions (throughout src/cleveragents/tool/builtins/file_tools.py):

  • _handle_file_read
  • _handle_file_write
  • _handle_file_edit
  • Other handler functions in the same file

Expected Behavior

All handler functions must validate arguments at function entry before any other logic:

def _handle_file_read(inputs: dict[str, Any]) -> dict[str, Any]:
    # Validate required arguments first (fail-fast)
    if "path" not in inputs:
        raise ValueError("Required argument 'path' is missing")
    if not isinstance(inputs["path"], str) or not inputs["path"]:
        raise ValueError("Argument 'path' must be a non-empty string")

    offset = inputs.get("offset", 0)
    if not isinstance(offset, int) or offset < 0:
        raise ValueError("Argument 'offset' must be a non-negative integer")

    limit = inputs.get("limit")
    if limit is not None and (not isinstance(limit, int) or limit <= 0):
        raise ValueError("Argument 'limit' must be a positive integer when provided")

    # ... then proceed with logic

Acceptance Criteria

  • All handler functions validate required arguments at function entry, before any other logic.
  • Clear ValueError messages are raised for all validation failures (not KeyError or TypeError).
  • Type validation is present for all typed arguments.
  • Range validation is present for all numeric arguments (offset ≥ 0, limit > 0 when provided).
  • Null/empty checks are present for all required string fields.
  • BDD unit tests (Behave) cover all validation scenarios including negative cases.
  • All nox stages pass and coverage ≥ 97%.

Supporting Information

  • File: src/cleveragents/tool/builtins/file_tools.py
  • Functions: _handle_file_read, _handle_file_write, _handle_file_edit, and other handler functions
  • Standard violated: CONTRIBUTING.md — Error and Exception Handling → Argument Validation: "All public and protected class methods validate arguments first: Value range, null checks, type verification, empty strings, empty collections, invalid states."
  • Impact: Functions fail with unclear KeyError/TypeError instead of actionable validation errors; violates architectural fail-fast principles; makes debugging harder when invalid inputs traverse the tool execution pipeline.

Backlog note: This issue was discovered during autonomous operation
on milestone v3.2.0. It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.


Metadata

  • Branch: bugfix/backlog-file-tools-argument-validation
  • Commit Message: fix(file-tools): add argument validation to file handler functions
  • Milestone: (backlog — no milestone assigned)
  • Parent Epic: (to be linked — see orphan note below)

Subtasks

  • Audit all handler functions in file_tools.py and enumerate every argument requiring validation
  • Add required-field existence checks ("path", "content", etc.) at function entry in each handler
  • Add type validation for all string arguments (path, content, encoding)
  • Add range validation for numeric arguments (offset ≥ 0, limit > 0 when provided)
  • Add null/empty checks for all required string fields
  • Write BDD Behave unit test scenarios covering all validation paths (positive and negative)
  • Ensure all nox stages pass with coverage ≥ 97%

Definition of Done

  • All handler functions validate required arguments at function entry before any other logic
  • Clear ValueError messages raised for all validation failures (no bare KeyError/TypeError)
  • Type validation present for all typed arguments
  • Range validation present for all numeric arguments
  • Null/empty checks present for all required string fields
  • BDD unit tests cover all validation scenarios including negative/edge cases
  • All nox stages pass
  • Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: new-issue-creator

## Background and Context The file tool handlers in `src/cleveragents/tool/builtins/file_tools.py` do not validate required arguments at function entry, violating the project's mandatory "fail-fast" error handling standards. Per CONTRIBUTING.md, all public and protected class methods must validate arguments first — before any other logic — covering value range, null checks, type verification, empty strings, and empty collections. ## Current Behavior Handler functions access required keys directly from the `inputs` dict without first verifying their existence, type, or value range: ```python def _handle_file_read(inputs: dict[str, Any]) -> dict[str, Any]: """Read file content.""" path = validate_path(inputs["path"], inputs.get("sandbox_root")) # ← No validation that "path" exists encoding: str = inputs.get("encoding", "utf-8") offset: int = inputs.get("offset", 0) # ← No validation that offset is non-negative limit: int | None = inputs.get("limit") # ← No validation that limit is positive def _handle_file_write(inputs: dict[str, Any]) -> dict[str, Any]: """Write content to file (create or overwrite).""" path = validate_path(inputs["path"], inputs.get("sandbox_root")) # ← No validation content: str = inputs["content"] # ← No validation that "content" exists or is string ``` **Specific missing validations:** 1. **Missing required field checks** — Functions access `inputs["path"]` and `inputs["content"]` without checking if the keys exist, causing opaque `KeyError` on missing input. 2. **No type validation** — No verification that values are the expected types (`str`, `int`, etc.), causing `TypeError` or silent misbehaviour. 3. **No range validation** — `offset` can be negative; `limit` can be zero or negative. 4. **No null/empty checks** — Fields can be `None` or empty strings without validation. **Affected functions** (throughout `src/cleveragents/tool/builtins/file_tools.py`): - `_handle_file_read` - `_handle_file_write` - `_handle_file_edit` - Other handler functions in the same file ## Expected Behavior All handler functions must validate arguments at function entry before any other logic: ```python def _handle_file_read(inputs: dict[str, Any]) -> dict[str, Any]: # Validate required arguments first (fail-fast) if "path" not in inputs: raise ValueError("Required argument 'path' is missing") if not isinstance(inputs["path"], str) or not inputs["path"]: raise ValueError("Argument 'path' must be a non-empty string") offset = inputs.get("offset", 0) if not isinstance(offset, int) or offset < 0: raise ValueError("Argument 'offset' must be a non-negative integer") limit = inputs.get("limit") if limit is not None and (not isinstance(limit, int) or limit <= 0): raise ValueError("Argument 'limit' must be a positive integer when provided") # ... then proceed with logic ``` ## Acceptance Criteria - All handler functions validate required arguments at function entry, before any other logic. - Clear `ValueError` messages are raised for all validation failures (not `KeyError` or `TypeError`). - Type validation is present for all typed arguments. - Range validation is present for all numeric arguments (`offset` ≥ 0, `limit` > 0 when provided). - Null/empty checks are present for all required string fields. - BDD unit tests (Behave) cover all validation scenarios including negative cases. - All `nox` stages pass and coverage ≥ 97%. ## Supporting Information - **File**: `src/cleveragents/tool/builtins/file_tools.py` - **Functions**: `_handle_file_read`, `_handle_file_write`, `_handle_file_edit`, and other handler functions - **Standard violated**: CONTRIBUTING.md — Error and Exception Handling → Argument Validation: *"All public and protected class methods validate arguments first: Value range, null checks, type verification, empty strings, empty collections, invalid states."* - **Impact**: Functions fail with unclear `KeyError`/`TypeError` instead of actionable validation errors; violates architectural fail-fast principles; makes debugging harder when invalid inputs traverse the tool execution pipeline. > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.2.0. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. --- ## Metadata - **Branch**: `bugfix/backlog-file-tools-argument-validation` - **Commit Message**: `fix(file-tools): add argument validation to file handler functions` - **Milestone**: *(backlog — no milestone assigned)* - **Parent Epic**: *(to be linked — see orphan note below)* ## Subtasks - [ ] Audit all handler functions in `file_tools.py` and enumerate every argument requiring validation - [ ] Add required-field existence checks (`"path"`, `"content"`, etc.) at function entry in each handler - [ ] Add type validation for all string arguments (`path`, `content`, `encoding`) - [ ] Add range validation for numeric arguments (`offset` ≥ 0, `limit` > 0 when provided) - [ ] Add null/empty checks for all required string fields - [ ] Write BDD Behave unit test scenarios covering all validation paths (positive and negative) - [ ] Ensure all `nox` stages pass with coverage ≥ 97% ## Definition of Done - [ ] All handler functions validate required arguments at function entry before any other logic - [ ] Clear `ValueError` messages raised for all validation failures (no bare `KeyError`/`TypeError`) - [ ] Type validation present for all typed arguments - [ ] Range validation present for all numeric arguments - [ ] Null/empty checks present for all required string fields - [ ] BDD unit tests cover all validation scenarios including negative/edge cases - [ ] All nox stages pass - [ ] Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: new-issue-creator
Author
Owner

⚠️ Orphan Issue — Manual Parent Epic Linking Required

This issue was created during autonomous bug-hunting and no suitable parent Epic could be identified at creation time. Per CONTRIBUTING.md, orphan issues are not permitted — every issue must be linked to a parent Epic via Forgejo's dependency system (child blocks parent).

Action required for a maintainer:

  1. Identify or create the appropriate parent Epic covering error handling / argument validation in the tool builtins layer.
  2. Link this issue as a child by running:
    curl -s -X POST "https://git.cleverthis.com/api/v1/repos/cleveragents/cleveragents-core/issues/7223/blocks" \
      -H "Authorization: token <YOUR_PAT>" \
      -H "Content-Type: application/json" \
      -d '{"owner": "cleveragents", "repo": "cleveragents-core", "index": <PARENT_EPIC_NUMBER>}'
    
  3. Assign a milestone once the parent Epic is identified and work is scheduled.

Suggested parent Epic candidates (related error-handling issues found during search):

  • #7169 — BUG-HUNT: [error-handling] Silent failures in PlanExecutor checkpoint operations hide critical errors
  • #6929 — BUG-HUNT: [spec-alignment] ErrorInfo.init missing required argument validation
  • #7039 — BUG-HUNT: [error-handling] Overly broad except Exception: patterns in CLI command modules mask critical errors

A broader "error handling standards compliance" Epic may be the most appropriate parent if one exists or is created.


Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: new-issue-creator

⚠️ **Orphan Issue — Manual Parent Epic Linking Required** This issue was created during autonomous bug-hunting and no suitable parent Epic could be identified at creation time. Per CONTRIBUTING.md, orphan issues are not permitted — every issue must be linked to a parent Epic via Forgejo's dependency system (child **blocks** parent). **Action required for a maintainer:** 1. Identify or create the appropriate parent Epic covering error handling / argument validation in the tool builtins layer. 2. Link this issue as a child by running: ```bash curl -s -X POST "https://git.cleverthis.com/api/v1/repos/cleveragents/cleveragents-core/issues/7223/blocks" \ -H "Authorization: token <YOUR_PAT>" \ -H "Content-Type: application/json" \ -d '{"owner": "cleveragents", "repo": "cleveragents-core", "index": <PARENT_EPIC_NUMBER>}' ``` 3. Assign a milestone once the parent Epic is identified and work is scheduled. **Suggested parent Epic candidates** (related error-handling issues found during search): - #7169 — BUG-HUNT: [error-handling] Silent failures in PlanExecutor checkpoint operations hide critical errors - #6929 — BUG-HUNT: [spec-alignment] ErrorInfo.__init__ missing required argument validation - #7039 — BUG-HUNT: [error-handling] Overly broad `except Exception:` patterns in CLI command modules mask critical errors A broader "error handling standards compliance" Epic may be the most appropriate parent if one exists or is created. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: new-issue-creator
Author
Owner

Verified — Critical bug: missing argument validation in file_tools.py. MoSCoW: Must-have. Priority: Critical.


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

✅ **Verified** — Critical bug: missing argument validation in file_tools.py. MoSCoW: Must-have. Priority: Critical. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor
Author
Owner

Verified — Critical bug: missing argument validation in file_tools.py. MoSCoW: Must-have. Priority: Critical.


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

✅ **Verified** — Critical bug: missing argument validation in file_tools.py. MoSCoW: Must-have. Priority: Critical. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor
Author
Owner

Verified — Critical bug: missing argument validation in file_tools.py. MoSCoW: Must-have. Priority: Critical.


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

✅ **Verified** — Critical bug: missing argument validation in file_tools.py. MoSCoW: Must-have. Priority: Critical. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
cleveragents/cleveragents-core#7223
No description provided.