BUG-HUNT: [concurrency] LockService uses ISO string comparison for timestamp ordering — timezone-naive strings can compare incorrectly #7407

New issue

Closed

opened 2026-04-10 19:04:47 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-10 19:04:47 +00:00

Owner

Bug Report: Data Integrity — Timestamp String Comparison in LockService

Severity Assessment

Impact: Expired locks may not be correctly detected as expired, leading to LockConflictError being raised when the lock should be considered stale and replaceable. Conversely, in edge cases near timezone offset boundaries, non-expired locks may be treated as expired.
Likelihood: Medium — affects any deployment using non-UTC system timezone or when ISO strings are stored with timezone suffixes that differ in formatting
Priority: Medium

Location

File: src/cleveragents/application/services/lock_service.py
Function: LockService.acquire(), LockService.renew(), LockService.cleanup_expired(), LockService.is_locked(), LockService.count_stale_locks()
Lines: Throughout (e.g. 159, 234, 278, 307, 328)

Description

The LockService stores all timestamps as ISO-format strings in the database and compares them using string comparison (>=, <). This is fragile:

datetime.now(tz=UTC).isoformat() produces strings like 2026-04-10T12:00:00+00:00
An older row stored with a slightly different format (e.g., 2026-04-10T12:00:00Z or 2026-04-10T12:00:00.000000+00:00) will sort incorrectly when compared as strings
String comparison of ISO timestamps is only reliable when ALL strings have the exact same format and timezone representation

Evidence

# src/cleveragents/application/services/lock_service.py
now = datetime.now(tz=UTC)
now_iso = now.isoformat()        # produces "2026-04-10T12:00:00.123456+00:00"
expires_iso = expires.isoformat()

# String comparison used for lock expiry check:
if existing_expires >= now_iso:  # line 159 — string comparison!
    raise LockConflictError(...)

# cleanup_expired:
stmt = delete(LockModel).where(LockModel.expires_at < now_iso)  # string comparison in SQL

Expected Behavior

Timestamp comparisons should use proper datetime objects or database-native datetime comparison, not string lexicographic ordering.

Actual Behavior

String comparison is used throughout — while Python's isoformat() generally produces consistent output for UTC timestamps, any legacy data, migration, or edge case could break the comparison silently.

Suggested Fix

Store timestamps as proper SQLAlchemy DateTime columns with timezone support (or Unix epoch integers)
Use func.now() in SQL queries for server-side time comparison
Or ensure consistent ISO format by always stripping microseconds: now.strftime('%Y-%m-%dT%H:%M:%S+00:00')

TDD Note

After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD with tags: @tdd_issue, @tdd_issue_, @tdd_expected_fail.

Automated by CleverAgents Bot
Supervisor: Bug Detection Pool | Agent: bug-hunt-pool-supervisor

## Bug Report: Data Integrity — Timestamp String Comparison in LockService ### Severity Assessment - **Impact**: Expired locks may not be correctly detected as expired, leading to `LockConflictError` being raised when the lock should be considered stale and replaceable. Conversely, in edge cases near timezone offset boundaries, non-expired locks may be treated as expired. - **Likelihood**: Medium — affects any deployment using non-UTC system timezone or when ISO strings are stored with timezone suffixes that differ in formatting - **Priority**: Medium ### Location - **File**: `src/cleveragents/application/services/lock_service.py` - **Function**: `LockService.acquire()`, `LockService.renew()`, `LockService.cleanup_expired()`, `LockService.is_locked()`, `LockService.count_stale_locks()` - **Lines**: Throughout (e.g. 159, 234, 278, 307, 328) ### Description The `LockService` stores all timestamps as ISO-format strings in the database and compares them using string comparison (`>=`, `<`). This is fragile: 1. `datetime.now(tz=UTC).isoformat()` produces strings like `2026-04-10T12:00:00+00:00` 2. An older row stored with a slightly different format (e.g., `2026-04-10T12:00:00Z` or `2026-04-10T12:00:00.000000+00:00`) will sort incorrectly when compared as strings 3. String comparison of ISO timestamps is only reliable when ALL strings have the exact same format and timezone representation ### Evidence ```python # src/cleveragents/application/services/lock_service.py now = datetime.now(tz=UTC) now_iso = now.isoformat() # produces "2026-04-10T12:00:00.123456+00:00" expires_iso = expires.isoformat() # String comparison used for lock expiry check: if existing_expires >= now_iso: # line 159 — string comparison! raise LockConflictError(...) # cleanup_expired: stmt = delete(LockModel).where(LockModel.expires_at < now_iso) # string comparison in SQL ``` ### Expected Behavior Timestamp comparisons should use proper datetime objects or database-native datetime comparison, not string lexicographic ordering. ### Actual Behavior String comparison is used throughout — while Python's `isoformat()` generally produces consistent output for UTC timestamps, any legacy data, migration, or edge case could break the comparison silently. ### Suggested Fix - Store timestamps as proper SQLAlchemy `DateTime` columns with timezone support (or Unix epoch integers) - Use `func.now()` in SQL queries for server-side time comparison - Or ensure consistent ISO format by always stripping microseconds: `now.strftime('%Y-%m-%dT%H:%M:%S+00:00')` ### Category data-flow ### TDD Note After this bug issue is verified, a corresponding Type/Testing issue will be created for TDD with tags: @tdd_issue, @tdd_issue_<this-issue-number>, @tdd_expected_fail. --- **Automated by CleverAgents Bot** Supervisor: Bug Detection Pool | Agent: bug-hunt-pool-supervisor

HAL9000 referenced this issue

2026-04-10 19:09:52 +00:00

[AUTO-BUG-SUP] Bug Hunt Status (Cycle 1) #7404

HAL9000 added the

labels

2026-04-10 19:28:42 +00:00

HAL9000 referenced this issue

2026-04-10 19:38:52 +00:00

[AUTO-BUG-SUP] Bug Hunt Status (Cycle 3) #7449

HAL9000 commented

2026-04-10 20:20:15 +00:00

Author

Owner

Closing as duplicate of #7341. Both issues describe the same bug: LockService uses ISO string comparison for timestamp ordering instead of proper datetime comparison. Issue #7341 was triaged first and assigned to v3.2.0 with Priority/Critical.

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

Closing as duplicate of #7341. Both issues describe the same bug: `LockService` uses ISO string comparison for timestamp ordering instead of proper datetime comparison. Issue #7341 was triaged first and assigned to v3.2.0 with Priority/Critical. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 closed this issue

2026-04-10 20:26:54 +00:00

HAL9000 referenced this issue

2026-04-10 20:39:58 +00:00

[AUTO-BUG-SUP] Bug Hunt Status (Cycle 4) #7470