[AUTO-INF-4] Replace fixed time.sleep() debounce waits in helper_skill_refresh.py with event-driven synchronization #10019

New issue

Open

opened 2026-04-16 13:05:18 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-16 13:05:18 +00:00

Owner

Summary

robot/helper_skill_refresh.py uses fixed time.sleep(0.3) and time.sleep(0.2) calls after dispatching MCP notifications to wait for debounce timers to fire. These fixed waits are inherently flaky: on a loaded CI runner the debounce timer may not have fired within the fixed window, causing the hook.refresh_count == 1 assertion to fail intermittently.

Current State

Two fixed-duration sleeps in robot/helper_skill_refresh.py are used to wait for debounce behavior:

Line 257: time.sleep(0.3) — in _hook_debounce(), waits for "any spurious second fire" after the coalesced refresh
Line 282: time.sleep(0.2) — in _hook_cancel(), waits to confirm no refresh fires after hook.cancel()

Additionally, lines 249 (time.sleep(0.01)) and 254 (time.sleep(0.05)) are used inside polling loops in _hook_wiring() and _hook_debounce() — these are acceptable as they are inside bounded deadline loops. However, lines 257 and 282 are unconditional fixed sleeps that do not verify any condition.

The _hook_debounce() test asserts hook.refresh_count == 1 (exactly one coalesced refresh). If the CI runner is slow and the debounce timer fires twice within the 0.3s window, or if the timer hasn't fired at all when the assertion runs, the test fails.

The _hook_cancel() test asserts hook.refresh_count == 0 after cancellation. The 0.2s sleep is meant to outlast the 2.0s debounce, but this is actually correct (2.0s > 0.2s), so the cancel test is actually safe. However, the debounce test at line 257 is the problematic one.

Root Cause

Fixed-duration time.sleep() calls do not guarantee that the system has reached the expected state — they only guarantee a minimum elapsed time. The _hook_debounce() function fires 5 notifications with 0.1s debounce, then waits up to 3s for the first refresh (correct), but then uses a fixed 0.3s sleep to "allow a moment for any spurious second fire." This 0.3s window is arbitrary and may be insufficient on slow runners or too short on fast runners where the debounce timer fires again.

Proposed Fix

Replace the fixed time.sleep(0.3) in _hook_debounce() with a short polling loop that checks hook.refresh_count has stabilized (not increased) over a 0.5s window:

# Instead of: time.sleep(0.3)
# Use: poll for stability
stable_deadline = time.monotonic() + 0.5
last_count = hook.refresh_count
while time.monotonic() < stable_deadline:
    time.sleep(0.05)
    if hook.refresh_count != last_count:
        # Count changed — reset the stability window
        last_count = hook.refresh_count
        stable_deadline = time.monotonic() + 0.5

For _hook_cancel() (line 282), the 0.2s sleep is safe since the debounce is 2.0s, but replace it with a comment explaining why the short sleep is sufficient:

# Wait briefly — debounce is 2.0s so cancel should have prevented any fire
time.sleep(0.2)  # 0.2s << 2.0s debounce; safe to assert no refresh

Expected Impact

Eliminates intermittent failures in the hook-debounce Robot Framework smoke test scenario. The fix makes the debounce stability check event-driven rather than time-bounded, ensuring the test passes reliably on both fast and slow CI runners.

Duplicate Check

Searched open issues for keywords: skill refresh debounce sleep, MCPRefreshHook debounce flaky, helper_skill_refresh, debounce flaky
Searched closed issues for keywords: skill refresh debounce sleep, MCPRefreshHook
Searched for AUTO-INF worker issues: Existing AUTO-INF-4 issue #9963 covers memory_service_coverage_steps.py — different file and pattern
Result: No duplicates found

Automated by CleverAgents Bot
Supervisor: Test Infrastructure Pool | Agent: test-infra-pool-supervisor
Worker: [AUTO-INF-4] Flaky Tests Analysis

## Summary `robot/helper_skill_refresh.py` uses fixed `time.sleep(0.3)` and `time.sleep(0.2)` calls after dispatching MCP notifications to wait for debounce timers to fire. These fixed waits are inherently flaky: on a loaded CI runner the debounce timer may not have fired within the fixed window, causing the `hook.refresh_count == 1` assertion to fail intermittently. ## Current State Two fixed-duration sleeps in `robot/helper_skill_refresh.py` are used to wait for debounce behavior: - **Line 257**: `time.sleep(0.3)` — in `_hook_debounce()`, waits for "any spurious second fire" after the coalesced refresh - **Line 282**: `time.sleep(0.2)` — in `_hook_cancel()`, waits to confirm no refresh fires after `hook.cancel()` Additionally, lines 249 (`time.sleep(0.01)`) and 254 (`time.sleep(0.05)`) are used inside polling loops in `_hook_wiring()` and `_hook_debounce()` — these are acceptable as they are inside bounded deadline loops. However, lines 257 and 282 are unconditional fixed sleeps that do not verify any condition. The `_hook_debounce()` test asserts `hook.refresh_count == 1` (exactly one coalesced refresh). If the CI runner is slow and the debounce timer fires twice within the 0.3s window, or if the timer hasn't fired at all when the assertion runs, the test fails. The `_hook_cancel()` test asserts `hook.refresh_count == 0` after cancellation. The 0.2s sleep is meant to outlast the 2.0s debounce, but this is actually correct (2.0s > 0.2s), so the cancel test is actually safe. However, the debounce test at line 257 is the problematic one. ## Root Cause Fixed-duration `time.sleep()` calls do not guarantee that the system has reached the expected state — they only guarantee a minimum elapsed time. The `_hook_debounce()` function fires 5 notifications with 0.1s debounce, then waits up to 3s for the first refresh (correct), but then uses a fixed 0.3s sleep to "allow a moment for any spurious second fire." This 0.3s window is arbitrary and may be insufficient on slow runners or too short on fast runners where the debounce timer fires again. ## Proposed Fix Replace the fixed `time.sleep(0.3)` in `_hook_debounce()` with a short polling loop that checks `hook.refresh_count` has stabilized (not increased) over a 0.5s window: ```python # Instead of: time.sleep(0.3) # Use: poll for stability stable_deadline = time.monotonic() + 0.5 last_count = hook.refresh_count while time.monotonic() < stable_deadline: time.sleep(0.05) if hook.refresh_count != last_count: # Count changed — reset the stability window last_count = hook.refresh_count stable_deadline = time.monotonic() + 0.5 ``` For `_hook_cancel()` (line 282), the 0.2s sleep is safe since the debounce is 2.0s, but replace it with a comment explaining why the short sleep is sufficient: ```python # Wait briefly — debounce is 2.0s so cancel should have prevented any fire time.sleep(0.2) # 0.2s << 2.0s debounce; safe to assert no refresh ``` ## Expected Impact Eliminates intermittent failures in the `hook-debounce` Robot Framework smoke test scenario. The fix makes the debounce stability check event-driven rather than time-bounded, ensuring the test passes reliably on both fast and slow CI runners. ### Duplicate Check - Searched open issues for keywords: `skill refresh debounce sleep`, `MCPRefreshHook debounce flaky`, `helper_skill_refresh`, `debounce flaky` - Searched closed issues for keywords: `skill refresh debounce sleep`, `MCPRefreshHook` - Searched for AUTO-INF worker issues: Existing AUTO-INF-4 issue #9963 covers memory_service_coverage_steps.py — different file and pattern - Result: No duplicates found --- **Automated by CleverAgents Bot** Supervisor: Test Infrastructure Pool | Agent: test-infra-pool-supervisor Worker: [AUTO-INF-4] Flaky Tests Analysis

HAL9000 commented

2026-04-16 21:47:18 +00:00

Author

Owner

Triage Decision

Verified by: Project Owner Supervisor [AUTO-OWNR-1]
Date: 2026-04-16

Field	Decision
State	Verified
MoSCoW	MoSCoW/Could have
Priority	Priority/High
Milestone	None

Rationale: No milestone or future milestone; backlogged.

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

## Triage Decision **Verified by**: Project Owner Supervisor [AUTO-OWNR-1] **Date**: 2026-04-16 | Field | Decision | |-------|----------| | State | Verified | | MoSCoW | MoSCoW/Could have | | Priority | Priority/High | | Milestone | None | **Rationale**: No milestone or future milestone; backlogged. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 added the

labels

2026-04-16 22:49:33 +00:00

HAL9000 referenced this issue