Skip to content

Commit 7607fa3

Browse files
masenfclaude
andauthored
fix: clean up stale asset symlinks to prevent hot reload crash loop (#6163)
* fix: clean up stale asset symlinks to prevent hot reload crash loop (#6159) When a Python module directory using rx.asset(shared=True) is renamed, stale symlinks in assets/external/ cause Granian to enter an infinite reload loop. Clean up broken symlinks and empty directories in assets/external/ in App.__call__ before compilation, so the cleanup runs on every hot reload, not just initial startup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * address PR review: guard rmdir against dir symlinks, fix fragile test - Add `not dirpath.is_symlink()` check before `rmdir()` to avoid NotADirectoryError on symlinks pointing to directories. - Use tmp_path/monkeypatch in no-external-dir test to avoid depending on prior test environment state. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * cr feedback: do not mutate dir while iterating over it * do not use cwd for tests --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 52a351c commit 7607fa3

File tree

3 files changed

+114
-7
lines changed

3 files changed

+114
-7
lines changed

reflex/app.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -615,8 +615,13 @@ def __call__(self) -> ASGIApp:
615615
Returns:
616616
The backend api.
617617
"""
618+
from reflex.assets import remove_stale_external_asset_symlinks
618619
from reflex.vars.base import GLOBAL_CACHE
619620

621+
# Clean up stale symlinks in assets/external/ before compiling, so that
622+
# rx.asset(shared=True) symlink re-creation doesn't trigger further reloads.
623+
remove_stale_external_asset_symlinks()
624+
620625
self._compile(prerender_routes=should_prerender_routes())
621626

622627
config = get_config()

reflex/assets.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,35 @@
77
from reflex.environment import EnvironmentVariables
88

99

10+
def remove_stale_external_asset_symlinks():
11+
"""Remove broken symlinks and empty directories in assets/external/.
12+
13+
When a Python module directory that uses rx.asset(shared=True) is renamed
14+
or deleted, stale symlinks remain in assets/external/ pointing to the old
15+
path. This cleanup prevents issues with file watchers detecting symlink
16+
re-creation during import.
17+
"""
18+
external_dir = (
19+
Path.cwd() / constants.Dirs.APP_ASSETS / constants.Dirs.EXTERNAL_APP_ASSETS
20+
)
21+
if not external_dir.exists():
22+
return
23+
24+
# Remove broken symlinks.
25+
broken = [
26+
p
27+
for p in external_dir.rglob("*")
28+
if p.is_symlink() and not p.resolve().exists()
29+
]
30+
for path in broken:
31+
path.unlink()
32+
33+
# Remove empty directories left behind (deepest first).
34+
for dirpath in sorted(external_dir.rglob("*"), reverse=True):
35+
if dirpath.is_dir() and not dirpath.is_symlink() and not any(dirpath.iterdir()):
36+
dirpath.rmdir()
37+
38+
1039
def asset(
1140
path: str,
1241
shared: bool = False,

tests/units/assets/test_assets.py

Lines changed: 80 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,40 @@
66

77
import reflex as rx
88
import reflex.constants as constants
9+
from reflex.assets import remove_stale_external_asset_symlinks
910

1011

11-
def test_shared_asset() -> None:
12+
@pytest.fixture
13+
def mock_asset_path(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
14+
"""Create a mock asset file and patch the current working directory.
15+
16+
Args:
17+
tmp_path: A temporary directory provided by pytest.
18+
monkeypatch: A pytest fixture for patching.
19+
20+
Returns:
21+
The path to a tmp cwd that will be used for assets.
22+
"""
23+
# Create a temporary directory to act as the current working directory.
24+
mock_cwd = tmp_path / "mock_asset_path"
25+
mock_cwd.mkdir()
26+
monkeypatch.chdir(mock_cwd)
27+
28+
return mock_cwd
29+
30+
31+
def test_shared_asset(mock_asset_path: Path) -> None:
1232
"""Test shared assets."""
1333
# The asset function copies a file to the app's external assets directory.
1434
asset = rx.asset(path="custom_script.js", shared=True, subfolder="subfolder")
1535
assert asset == "/external/test_assets/subfolder/custom_script.js"
1636
result_file = Path(
17-
Path.cwd(), "assets/external/test_assets/subfolder/custom_script.js"
37+
mock_asset_path,
38+
"assets",
39+
"external",
40+
"test_assets",
41+
"subfolder",
42+
"custom_script.js",
1843
)
1944
assert result_file.exists()
2045

@@ -24,17 +49,19 @@ def test_shared_asset() -> None:
2449
# Test the asset function without a subfolder.
2550
asset = rx.asset(path="custom_script.js", shared=True)
2651
assert asset == "/external/test_assets/custom_script.js"
27-
result_file = Path(Path.cwd(), "assets/external/test_assets/custom_script.js")
52+
result_file = Path(
53+
mock_asset_path, "assets", "external", "test_assets", "custom_script.js"
54+
)
2855
assert result_file.exists()
2956

3057
# clean up
31-
shutil.rmtree(Path.cwd() / "assets/external")
58+
shutil.rmtree(Path(mock_asset_path) / "assets" / "external")
3259

3360
with pytest.raises(FileNotFoundError):
3461
asset = rx.asset("non_existent_file.js")
3562

3663
# Nothing is done to assets when file does not exist.
37-
assert not Path(Path.cwd() / "assets/external").exists()
64+
assert not Path(mock_asset_path / "assets" / "external").exists()
3865

3966

4067
@pytest.mark.parametrize(
@@ -56,13 +83,13 @@ def test_invalid_assets(path: str, shared: bool) -> None:
5683

5784

5885
@pytest.fixture
59-
def custom_script_in_asset_dir() -> Generator[Path, None, None]:
86+
def custom_script_in_asset_dir(mock_asset_path: Path) -> Generator[Path, None, None]:
6087
"""Create a custom_script.js file in the app's assets directory.
6188
6289
Yields:
6390
The path to the custom_script.js file.
6491
"""
65-
asset_dir = Path.cwd() / constants.Dirs.APP_ASSETS
92+
asset_dir = mock_asset_path / constants.Dirs.APP_ASSETS
6693
asset_dir.mkdir(exist_ok=True)
6794
path = asset_dir / "custom_script.js"
6895
path.touch()
@@ -79,3 +106,49 @@ def test_local_asset(custom_script_in_asset_dir: Path) -> None:
79106
"""
80107
asset = rx.asset("custom_script.js", shared=False)
81108
assert asset == "/custom_script.js"
109+
110+
111+
def test_remove_stale_external_asset_symlinks(mock_asset_path: Path) -> None:
112+
"""Test that stale symlinks and empty dirs in assets/external/ are cleaned up."""
113+
external_dir = (
114+
mock_asset_path / constants.Dirs.APP_ASSETS / constants.Dirs.EXTERNAL_APP_ASSETS
115+
)
116+
117+
# Set up: create a subdirectory with a broken symlink.
118+
stale_dir = external_dir / "old_module" / "subpkg"
119+
stale_dir.mkdir(parents=True, exist_ok=True)
120+
stale_symlink = stale_dir / "missing_file.js"
121+
stale_symlink.symlink_to("/nonexistent/path/missing_file.js")
122+
assert stale_symlink.is_symlink()
123+
assert not stale_symlink.resolve().exists()
124+
125+
# Also create a valid symlink that should be preserved.
126+
valid_dir = external_dir / "valid_module"
127+
valid_dir.mkdir(parents=True, exist_ok=True)
128+
valid_target = Path(__file__).parent / "custom_script.js"
129+
valid_symlink = valid_dir / "custom_script.js"
130+
valid_symlink.symlink_to(valid_target)
131+
assert valid_symlink.is_symlink()
132+
assert valid_symlink.resolve().exists()
133+
134+
remove_stale_external_asset_symlinks()
135+
136+
# Broken symlink and its empty parent dirs should be removed.
137+
assert not stale_symlink.exists()
138+
assert not stale_symlink.is_symlink()
139+
assert not stale_dir.exists()
140+
assert not (external_dir / "old_module").exists()
141+
142+
# Valid symlink should be preserved.
143+
assert valid_symlink.is_symlink()
144+
assert valid_symlink.resolve().exists()
145+
146+
147+
def test_remove_stale_symlinks_no_external_dir(mock_asset_path: Path) -> None:
148+
"""Test that cleanup is a no-op when assets/external/ doesn't exist."""
149+
external_dir = (
150+
mock_asset_path / constants.Dirs.APP_ASSETS / constants.Dirs.EXTERNAL_APP_ASSETS
151+
)
152+
assert not external_dir.exists()
153+
# Should not raise.
154+
remove_stale_external_asset_symlinks()

0 commit comments

Comments
 (0)