[Amadeus-22]

Description

safe_save retried with safe_save(data, path, ++_retry). In Python ++_retry is a double unary plus, not an increment, so the counter stays at 0 and the _retry < 10 limit is never reached. When the lock file never looks stale (another process keeps refreshing it, or its mtime is in the future), safe_save calls itself until RecursionError.

This passes _retry + 1, so it gives up after 10 retries as intended.

How Has This Been Tested?

New test test_safe_save_gives_up_after_retries_when_the_lock_file_is_never_released: a lock file with a future mtime, time.sleep mocked. On master it fails (it recurses for over a minute before erroring); with the fix it retries 11 times and returns. pytest tests/components/config/ passes (100 passed, 2 skipped) on Python 3.12.

🤖 Generated with Claude Code

[quantconnect-kronos-reviewer]

Approving. This fixes the retry counter in safe_save and does nothing else.

The fix. In Python, ++_retry applies unary plus twice and leaves the value unchanged. Every recursive call got _retry == 0, so the _retry < 10 guard never stopped anything. A lock that never goes stale (for example, its mtime is in the future) made safe_save recurse until RecursionError. With the default recursion limit and 0.25 s sleeps, that takes about 4 minutes. _retry + 1 makes the code do what it has meant to do since the lock was added in 9ca3cac: give up after 10 retries.

What else this changes. I traced the paths that already work:

• No contention: the first open(lock_file, 'x') succeeds, so the changed line never runs.
• Stale lock left by a crashed process: the first retry removes it (mtime is 2 s or older) and the next attempt gets the lock.
• Lock held by a live writer: 11 attempts with ~0.25 s sleeps cover about 2.75 s. The 2 s staleness rule removes the lock by about the 9th attempt, so the retry budget still covers the normal wait.
• Callers: Storage._save, lean_config_manager, init and pull_manager all lock per file path. A single lean cloud pull writes distinct paths, so it doesn't contend with itself.

Test. The new test reaches the bug's exact condition: a lock file with a future mtime, with time.sleep patched. safe_save imports sleep inside the function, so patching time.sleep takes effect. With the fix there are 11 attempts and 11 sleeps, so call_count == 11 is right. Without the fix the test would recurse until it errors. It runs on the autouse fake filesystem, so no real files are touched.

CI. All green: Ubuntu, macOS and Windows on Python 3.9 to 3.14.

Not blocking, could be its own issue. After the 10 retries, safe_save returns without a warning, and the test asserts that by checking the file doesn't exist. That was the original design, and it's better than a stack-depth crash. A follow-up could log or raise when the write is dropped, so a stuck lock doesn't silently stop a setting from being saved.

Risk: risk:low. It's a one-line change, easy to revert, and it only affects the retry branch, which before crashed with RecursionError. It has a regression test and green CI on every platform. An end-to-end CLI run wouldn't touch this branch unless a lock is deliberately stuck, so the unit test is the relevant evidence.

Other repositories: none affected.