From 8229c1165d241aa0bd1cac2de61d08066384efbb Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Wed, 22 Jul 2026 01:00:08 +0000 Subject: [PATCH 1/3] throttle: support per-key windows and a reset hook The throttle decorator kept a single last_call timestamp shared across all invocations, so the window was global per decorated function. Add an optional key function so each key gets its own window; key=None preserves the old global behaviour. Store windows in a dict and expose wrapper.reset() to clear them (used by tests that would otherwise inherit a stale window across cases). Co-Authored-By: Claude Opus 4.8 (1M context) --- shard_core/util/misc.py | 22 ++++++++++++++-------- tests/test_throttle.py | 22 ++++++++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/shard_core/util/misc.py b/shard_core/util/misc.py index 52537588..293c50b1 100644 --- a/shard_core/util/misc.py +++ b/shard_core/util/misc.py @@ -2,26 +2,32 @@ import time -def throttle(min_duration: float): +def throttle(min_duration: float, key=None): def decorator_throttle(func): - last_call = None + last_call: dict = {} + + def is_allowed(*args, **kwargs): + k = key(*args, **kwargs) if key is not None else None + now = time.time() + prev = last_call.get(k) + if prev is None or prev + min_duration < now: + last_call[k] = now + return True + return False if inspect.iscoroutinefunction(func): async def wrapper_throttle(*args, **kwargs): - nonlocal last_call - if last_call is None or last_call + min_duration < time.time(): - last_call = time.time() + if is_allowed(*args, **kwargs): return await func(*args, **kwargs) else: def wrapper_throttle(*args, **kwargs): - nonlocal last_call - if last_call is None or last_call + min_duration < time.time(): - last_call = time.time() + if is_allowed(*args, **kwargs): return func(*args, **kwargs) + wrapper_throttle.reset = last_call.clear return wrapper_throttle return decorator_throttle diff --git a/tests/test_throttle.py b/tests/test_throttle.py index df1e9e8b..127f970f 100644 --- a/tests/test_throttle.py +++ b/tests/test_throttle.py @@ -28,3 +28,25 @@ def call_me(): assert call_me() == "called" time.sleep(0.2) assert call_me() == "called" + + +def test_throttle_per_key_windows_are_independent(): + @throttle(0.1, key=lambda name: name) + def call_me(name): + return f"called {name}" + + assert call_me("a") == "called a" + assert call_me("b") == "called b" + assert call_me("a") is None + assert call_me("b") is None + + +def test_throttle_reset_clears_all_windows(): + @throttle(0.1, key=lambda name: name) + def call_me(name): + return f"called {name}" + + assert call_me("a") == "called a" + assert call_me("a") is None + call_me.reset() + assert call_me("a") == "called a" From a077fddace89c0117bca89ba0beb0fc79feef385 Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Wed, 22 Jul 2026 01:00:18 +0000 Subject: [PATCH 2/3] docker_start_app: throttle per app name, not globally @throttle(5) on docker_start_app was global, so a request to start app B within 5s of app A starting was silently dropped and B stayed on its splash until the next request or lifecycle tick. Key the throttle on the app name so each app has its own 5s dedupe window. Update the reset fixture to the new reset() hook and fix the now-stale "global" wording in app_lifecycle. Co-Authored-By: Claude Opus 4.8 (1M context) --- shard_core/service/app_lifecycle.py | 2 +- shard_core/service/app_tools.py | 2 +- tests/test_app_compose_pinning.py | 44 +++++++++++++++++++++++++---- 3 files changed, 41 insertions(+), 7 deletions(-) diff --git a/shard_core/service/app_lifecycle.py b/shard_core/service/app_lifecycle.py index 79e690e7..2a59c533 100644 --- a/shard_core/service/app_lifecycle.py +++ b/shard_core/service/app_lifecycle.py @@ -37,7 +37,7 @@ async def ensure_app_is_running(app: InstalledApp): last_access_dict[app.name] = time.time() # PAUSED wakes via unpause (ms to ~2s), everything else via the legacy # start path. Calling docker_unpause_app directly also dodges the - # global 5s throttle on docker_start_app, which would silently drop + # per-app 5s throttle on docker_start_app, which would silently drop # the wake. This dispatch stays active even with pause_enabled=false, # so already-paused apps still wake after a backout. if app.status == Status.PAUSED: diff --git a/shard_core/service/app_tools.py b/shard_core/service/app_tools.py index b7fdc8cf..687ebaa1 100644 --- a/shard_core/service/app_tools.py +++ b/shard_core/service/app_tools.py @@ -31,7 +31,7 @@ async def docker_create_app_containers(name: str): await subprocess(*_app_compose(name), "up", "--no-start") -@throttle(5) +@throttle(5, key=lambda name, *a, **kw: name) async def docker_start_app(name: str): async with db_conn() as conn: app = await db_installed_apps.get_by_name(conn, name) diff --git a/tests/test_app_compose_pinning.py b/tests/test_app_compose_pinning.py index 8b2793bf..101c00bd 100644 --- a/tests/test_app_compose_pinning.py +++ b/tests/test_app_compose_pinning.py @@ -95,11 +95,9 @@ def subprocess_mock(): @pytest.fixture(autouse=True) def reset_start_throttle(): - """docker_start_app's @throttle(5) is global across apps and tests — a start - triggered by an earlier test would silently drop our call.""" - wrapper = app_tools.docker_start_app - cell = wrapper.__closure__[wrapper.__code__.co_freevars.index("last_call")] - cell.cell_contents = None + """docker_start_app's @throttle(5) is per-app but persists across tests — a + start of the same app in an earlier test would silently drop our call.""" + app_tools.docker_start_app.reset() async def _insert_app(name: str, status: Status): @@ -148,3 +146,39 @@ async def test_app_operation_with_compose_file_pins_the_app_project( assert "-f" in command and command[command.index("-f") + 1] == str( app_dir / "docker-compose.yml" ) + + +def _started_apps(subprocess_mock) -> list[str]: + return [ + c.args[c.args.index("-p") + 1] + for c in subprocess_mock.call_args_list + if c.args[-2:] == ("up", "-d") + ] + + +async def test_docker_start_app_starts_each_app_despite_throttle( + db, tmp_path, subprocess_mock +): + _app_dir(tmp_path, "appa") + _app_dir(tmp_path, "appb") + await _insert_app("appa", Status.STOPPED) + await _insert_app("appb", Status.STOPPED) + + with settings_override({"path_root": str(tmp_path)}): + await app_tools.docker_start_app("appa") + await app_tools.docker_start_app("appb") + + assert _started_apps(subprocess_mock) == ["appa", "appb"] + + +async def test_docker_start_app_throttles_repeated_start_of_same_app( + db, tmp_path, subprocess_mock +): + _app_dir(tmp_path, "appa") + await _insert_app("appa", Status.STOPPED) + + with settings_override({"path_root": str(tmp_path)}): + await app_tools.docker_start_app("appa") + await app_tools.docker_start_app("appa") + + assert _started_apps(subprocess_mock) == ["appa"] From 2f45a37286536b456311a323e0ec39ab983f06a3 Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Wed, 22 Jul 2026 01:10:14 +0000 Subject: [PATCH 3/3] throttle: document key/reset, simplify key, harden repeat-start test Review panel follow-ups: document the throttle key/reset contract; drop the unreachable *args/**kwargs from the app-name key lambda (both callers pass name positionally); and in the repeat-start test reset the app to a startable status between calls so the throttle, not status gating, is the sole reason the second start is dropped. Co-Authored-By: Claude Opus 4.8 (1M context) --- shard_core/service/app_tools.py | 2 +- shard_core/util/misc.py | 7 +++++++ tests/test_app_compose_pinning.py | 4 ++++ 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/shard_core/service/app_tools.py b/shard_core/service/app_tools.py index 687ebaa1..1d304abd 100644 --- a/shard_core/service/app_tools.py +++ b/shard_core/service/app_tools.py @@ -31,7 +31,7 @@ async def docker_create_app_containers(name: str): await subprocess(*_app_compose(name), "up", "--no-start") -@throttle(5, key=lambda name, *a, **kw: name) +@throttle(5, key=lambda name: name) async def docker_start_app(name: str): async with db_conn() as conn: app = await db_installed_apps.get_by_name(conn, name) diff --git a/shard_core/util/misc.py b/shard_core/util/misc.py index 293c50b1..5db1732c 100644 --- a/shard_core/util/misc.py +++ b/shard_core/util/misc.py @@ -3,6 +3,13 @@ def throttle(min_duration: float, key=None): + """Drop calls that arrive within min_duration of the previous accepted call. + + key maps the call args to a bucket key so each bucket throttles + independently; key=None (default) uses one global window for the function. + The wrapper exposes reset() to clear all windows (test hook). + """ + def decorator_throttle(func): last_call: dict = {} diff --git a/tests/test_app_compose_pinning.py b/tests/test_app_compose_pinning.py index 101c00bd..2e5c4626 100644 --- a/tests/test_app_compose_pinning.py +++ b/tests/test_app_compose_pinning.py @@ -179,6 +179,10 @@ async def test_docker_start_app_throttles_repeated_start_of_same_app( with settings_override({"path_root": str(tmp_path)}): await app_tools.docker_start_app("appa") + # force back to a startable status so only the throttle, not status + # gating, can drop the second start + async with db_conn() as conn: + await db_installed_apps.update_status(conn, "appa", Status.STOPPED) await app_tools.docker_start_app("appa") assert _started_apps(subprocess_mock) == ["appa"]