cleveragents/cleveragents-core

Fork 3

[Bug Hunt][Cycle 2][Config] File I/O race conditions in TOML config operations #7106

New issue

Open

opened 2026-04-10 07:45:23 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-10 07:45:23 +00:00

Owner

Background

The ConfigService in src/cleveragents/application/services/config_service.py performs non-atomic read-modify-write operations on TOML configuration files without any file locking mechanism. In production environments where multiple CleverAgents processes or threads may access the same configuration file simultaneously, this creates race conditions that can result in configuration file corruption or silent data loss.

This is a concurrency correctness issue affecting the core configuration subsystem. The agents config set CLI command and any internal service that calls ConfigService.write_config(), ConfigService.write_scoped_config(), or ConfigService.set_value() are all affected.

Current Behavior

The three write methods all follow the same non-atomic read-modify-write pattern:

def write_config(self, data: dict[str, Any]) -> None:
    self._config_dir.mkdir(parents=True, exist_ok=True)

    if self._config_path.exists():
        with open(self._config_path) as fh:
            doc = tomlkit.load(fh)  # Step 1: Read
    else:
        doc = tomlkit.document()

    for key, value in data.items():
        doc[key] = value            # Step 2: Modify

    with open(self._config_path, "w") as fh:
        tomlkit.dump(doc, fh)       # Step 3: Write (overwrites entire file)

Race condition scenario:

Process A reads the config file (Step 1)
Process B reads the same config file (Step 1)
Process B modifies and writes its changes (Steps 2–3)
Process A writes its changes, overwriting Process B's changes (Steps 2–3)

This results in lost writes and potentially corrupted configuration state. The same pattern exists in write_scoped_config() (lines 1277–1320) and set_value() (lines 1322–1366).

Affected locations:

src/cleveragents/application/services/config_service.py
- ConfigService.write_config() — lines 1261–1275
- ConfigService.write_scoped_config() — lines 1277–1320
- ConfigService.set_value() — lines 1322–1366

Expected Behavior

Configuration file updates must be atomic. The entire read-modify-write sequence must be protected by a file lock so that only one process can modify the file at a time. No writes should be silently lost when multiple processes update configuration concurrently.

Acceptance Criteria

All three write methods (write_config, write_scoped_config, set_value) acquire an exclusive file lock before reading and release it only after writing
File locking is cross-platform: fcntl.flock() on Unix/macOS, msvcrt.locking() on Windows (or a cross-platform library such as filelock)
Writes use an atomic rename pattern (write to a temp file, then os.replace()) to prevent partial writes on crash
No configuration data is lost when two processes call set_value() concurrently on the same key or different keys
All existing nox stages pass
Coverage remains ≥ 97%

Supporting Information

Suggested fix approach:

import fcntl  # Unix; use filelock for cross-platform

def write_config(self, data: dict[str, Any]) -> None:
    self._config_dir.mkdir(parents=True, exist_ok=True)
    lock_path = self._config_path.with_suffix(".lock")
    with open(lock_path, "w") as lock_fh:
        fcntl.flock(lock_fh, fcntl.LOCK_EX)
        try:
            if self._config_path.exists():
                with open(self._config_path) as fh:
                    doc = tomlkit.load(fh)
            else:
                doc = tomlkit.document()
            for key, value in data.items():
                doc[key] = value
            tmp_path = self._config_path.with_suffix(".tmp")
            with open(tmp_path, "w") as fh:
                tomlkit.dump(doc, fh)
            os.replace(tmp_path, self._config_path)
        finally:
            fcntl.flock(lock_fh, fcntl.LOCK_UN)

A cross-platform alternative is the filelock library (already a common Python dependency).

TDD Note: After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD. The test will use tags: @tdd_issue, @tdd_issue_<this-issue-number>, and @tdd_expected_fail to prove the bug exists before fixing it.

Metadata

Branch: bugfix/m3-config-toml-file-io-race-conditions
Commit Message: fix(config): add file locking to prevent race conditions in TOML write operations
Milestone: v3.2.0
Parent Epic: #5502

Subtasks

Reproduce the race condition with a concurrent-write test (TDD issue to be created separately)
Implement cross-platform file locking wrapper (Unix fcntl.flock / Windows msvcrt or filelock)
Refactor write_config() to use lock + atomic rename pattern
Refactor write_scoped_config() to use lock + atomic rename pattern
Refactor set_value() to use lock + atomic rename pattern
Verify no deadlock when the same process calls nested write methods
Update docstrings to document thread/process safety guarantees
Run nox — all stages must pass
Confirm coverage ≥ 97%

Definition of Done

All three write methods are protected by an exclusive file lock
Atomic rename (os.replace) used to prevent partial-write corruption
Cross-platform locking strategy implemented and documented
@tdd_expected_fail tag removed from the TDD regression test (proving the fix works)
@tdd_issue and @tdd_issue_<N> tags remain permanently on the regression test
All nox stages pass
Coverage >= 97%
PR submitted, reviewed, and merged with Closes #<this-issue-number> in footer

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

## Background The `ConfigService` in `src/cleveragents/application/services/config_service.py` performs non-atomic read-modify-write operations on TOML configuration files without any file locking mechanism. In production environments where multiple CleverAgents processes or threads may access the same configuration file simultaneously, this creates race conditions that can result in configuration file corruption or silent data loss. This is a concurrency correctness issue affecting the core configuration subsystem. The `agents config set` CLI command and any internal service that calls `ConfigService.write_config()`, `ConfigService.write_scoped_config()`, or `ConfigService.set_value()` are all affected. ## Current Behavior The three write methods all follow the same non-atomic read-modify-write pattern: ```python def write_config(self, data: dict[str, Any]) -> None: self._config_dir.mkdir(parents=True, exist_ok=True) if self._config_path.exists(): with open(self._config_path) as fh: doc = tomlkit.load(fh) # Step 1: Read else: doc = tomlkit.document() for key, value in data.items(): doc[key] = value # Step 2: Modify with open(self._config_path, "w") as fh: tomlkit.dump(doc, fh) # Step 3: Write (overwrites entire file) ``` **Race condition scenario:** 1. Process A reads the config file (Step 1) 2. Process B reads the same config file (Step 1) 3. Process B modifies and writes its changes (Steps 2–3) 4. Process A writes its changes, **overwriting Process B's changes** (Steps 2–3) This results in lost writes and potentially corrupted configuration state. The same pattern exists in `write_scoped_config()` (lines 1277–1320) and `set_value()` (lines 1322–1366). **Affected locations:** - `src/cleveragents/application/services/config_service.py` - `ConfigService.write_config()` — lines 1261–1275 - `ConfigService.write_scoped_config()` — lines 1277–1320 - `ConfigService.set_value()` — lines 1322–1366 ## Expected Behavior Configuration file updates must be atomic. The entire read-modify-write sequence must be protected by a file lock so that only one process can modify the file at a time. No writes should be silently lost when multiple processes update configuration concurrently. ## Acceptance Criteria - All three write methods (`write_config`, `write_scoped_config`, `set_value`) acquire an exclusive file lock before reading and release it only after writing - File locking is cross-platform: `fcntl.flock()` on Unix/macOS, `msvcrt.locking()` on Windows (or a cross-platform library such as `filelock`) - Writes use an atomic rename pattern (write to a temp file, then `os.replace()`) to prevent partial writes on crash - No configuration data is lost when two processes call `set_value()` concurrently on the same key or different keys - All existing `nox` stages pass - Coverage remains ≥ 97% ## Supporting Information **Suggested fix approach:** ```python import fcntl # Unix; use filelock for cross-platform def write_config(self, data: dict[str, Any]) -> None: self._config_dir.mkdir(parents=True, exist_ok=True) lock_path = self._config_path.with_suffix(".lock") with open(lock_path, "w") as lock_fh: fcntl.flock(lock_fh, fcntl.LOCK_EX) try: if self._config_path.exists(): with open(self._config_path) as fh: doc = tomlkit.load(fh) else: doc = tomlkit.document() for key, value in data.items(): doc[key] = value tmp_path = self._config_path.with_suffix(".tmp") with open(tmp_path, "w") as fh: tomlkit.dump(doc, fh) os.replace(tmp_path, self._config_path) finally: fcntl.flock(lock_fh, fcntl.LOCK_UN) ``` A cross-platform alternative is the `filelock` library (already a common Python dependency). **TDD Note:** After this bug issue is verified, a corresponding `Type/Testing` issue will be created for TDD. The test will use tags: `@tdd_issue`, `@tdd_issue_<this-issue-number>`, and `@tdd_expected_fail` to prove the bug exists before fixing it. ## Metadata - **Branch**: `bugfix/m3-config-toml-file-io-race-conditions` - **Commit Message**: `fix(config): add file locking to prevent race conditions in TOML write operations` - **Milestone**: v3.2.0 - **Parent Epic**: #5502 ## Subtasks - [ ] Reproduce the race condition with a concurrent-write test (TDD issue to be created separately) - [ ] Implement cross-platform file locking wrapper (Unix `fcntl.flock` / Windows `msvcrt` or `filelock`) - [ ] Refactor `write_config()` to use lock + atomic rename pattern - [ ] Refactor `write_scoped_config()` to use lock + atomic rename pattern - [ ] Refactor `set_value()` to use lock + atomic rename pattern - [ ] Verify no deadlock when the same process calls nested write methods - [ ] Update docstrings to document thread/process safety guarantees - [ ] Run `nox` — all stages must pass - [ ] Confirm coverage ≥ 97% ## Definition of Done - [ ] All three write methods are protected by an exclusive file lock - [ ] Atomic rename (`os.replace`) used to prevent partial-write corruption - [ ] Cross-platform locking strategy implemented and documented - [ ] `@tdd_expected_fail` tag removed from the TDD regression test (proving the fix works) - [ ] `@tdd_issue` and `@tdd_issue_<N>` tags remain permanently on the regression test - [ ] All nox stages pass - [ ] Coverage >= 97% - [ ] PR submitted, reviewed, and merged with `Closes #<this-issue-number>` in footer --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: new-issue-creator

HAL9000 added this to the v3.2.0 milestone

2026-04-10 07:46:00 +00:00

HAL9000 added the

State

Unverified

label

2026-04-10 07:50:07 +00:00

HAL9000 added a new dependency

2026-04-10 07:50:28 +00:00

#5502 EPIC: Actor Execution & Configuration — GRAPH Actor, Config Schema & CLI Compliance (v3.5.0)

HAL9000 referenced this issue

2026-04-10 08:37:30 +00:00

[AUTO-BUG-POOL] Bug Hunt Worker 10 Summary Report #7183

freemo added the

labels

2026-04-12 18:37:13 +00:00

HAL9000 added

and removed

labels

2026-04-14 06:48:49 +00:00

HAL9000 commented

2026-04-14 06:48:50 +00:00

Author

Owner

✅ Verified — Critical concurrency bug: file I/O race conditions in TOML config operations. MoSCoW: Must-have. Priority: Critical.

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

✅ **Verified** — Critical concurrency bug: file I/O race conditions in TOML config operations. MoSCoW: Must-have. Priority: Critical. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor