fix: clean up stale asset symlinks to prevent hot reload crash loop

[masenf]

fix #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.

[greptile-apps]

Greptile Summary

This PR fixes a hot-reload crash loop (issue #6159) where renaming a Python module that uses rx.asset(shared=True) leaves stale symlinks in assets/external/, causing Granian's file watcher to continuously detect changes and re-trigger reloads. The fix adds a remove_stale_external_asset_symlinks() utility that cleans up broken symlinks and their empty parent directories before each compilation cycle in App.__call__.

Key changes:

• reflex/assets.py: New remove_stale_external_asset_symlinks() function walks assets/external/, unlinks broken symlinks, then removes empty directories deepest-first using a reverse-sorted rglob.
• reflex/app.py: Calls the cleanup at the top of App.__call__ (before _compile), so it runs on every hot-reload, not just initial startup.
• tests/units/assets/test_assets.py: Two new tests cover the happy path (stale symlink removed, valid symlink preserved) and the no-op path (missing external_dir).

Issues found:

• test_remove_stale_external_asset_symlinks creates and destroys assets/external/ relative to the real Path.cwd() rather than a tmp_path. The shutil.rmtree in its finally block would silently wipe real app assets if the tests are run from inside a Reflex project directory. Using monkeypatch.chdir(tmp_path) (as done in the companion test) would make it fully hermetic.
• The broken-symlink removal loop mutates the directory tree while a live rglob generator is still open; collecting candidates into a list first is a minor robustness improvement.

Confidence Score: 4/5

• Safe to merge after addressing the test isolation issue; the production logic is sound.
• The core fix in assets.py and the call-site change in app.py are correct and well-scoped. The only meaningful concern is in the test — it uses the real CWD rather than a temporary directory, which could destroy real assets/external/ content in certain environments. This does not affect production behaviour but is worth fixing before merging to avoid CI environment pollution.
• tests/units/assets/test_assets.py — the main test function needs monkeypatch.chdir(tmp_path) to be environment-safe.

Important Files Changed

Filename	Overview
reflex/assets.py	Adds remove_stale_external_asset_symlinks() that walks assets/external/, unlinks broken symlinks, and removes now-empty directories; logic is correct (including the not dirpath.is_symlink() guard), but mutating the directory tree while holding a live rglob generator is a minor robustness concern.
reflex/app.py	Calls remove_stale_external_asset_symlinks() at the start of App.__call__ (before _compile), ensuring cleanup runs on every hot-reload cycle; the placement is correct and the lazy import avoids circular-import issues.
tests/units/assets/test_assets.py	Adds two new tests for the cleanup function; the test_remove_stale_external_asset_symlinks test is missing monkeypatch.chdir(tmp_path), meaning it operates on the real CWD and shutil.rmtree in its finally block can destroy real app assets; test_remove_stale_symlinks_no_external_dir is implemented correctly.

Sequence Diagram

sequenceDiagram
    participant Granian as Granian (file watcher)
    participant App as App.__call__
    participant Cleanup as remove_stale_external_asset_symlinks
    participant FS as assets/external/ (filesystem)
    participant Compile as App._compile

    Granian->>App: hot reload triggered
    App->>Cleanup: remove_stale_external_asset_symlinks()
    Cleanup->>FS: rglob("*") — find all paths
    FS-->>Cleanup: list of paths (symlinks + dirs)
    Cleanup->>FS: unlink broken symlinks (target missing)
    Cleanup->>FS: rglob("*") — find remaining dirs
    FS-->>Cleanup: remaining directories
    Cleanup->>FS: rmdir() empty directories (deepest first)
    Cleanup-->>App: done
    App->>Compile: _compile(prerender_routes=...)
    Compile->>FS: rx.asset(shared=True) recreates symlinks
    Note over FS: No stale symlinks → no spurious<br/>file-change events → no reload loop
    Compile-->>App: compilation complete
    App-->>Granian: ASGI app returned

Loading

_{Last reviewed commit: ff962fc}

[greptile-apps]

rmdir() will fail on valid symlinks pointing to directories

Path.is_dir() returns True for both real directories and symlinks that resolve to a directory. If assets/external/ ever contains a valid directory symlink (e.g., placed there manually or via a future API change), dirpath.rmdir() would raise NotADirectoryError on POSIX systems because os.rmdir() requires the path to be an actual directory, not a symlink.

The check should also exclude symlinks:

Suggested change

	for dirpath in sorted(external_dir.rglob("*"), reverse=True):
	if dirpath.is_dir() and not any(dirpath.iterdir()):
	dirpath.rmdir()
	for dirpath in sorted(external_dir.rglob("*"), reverse=True):
	if dirpath.is_dir() and not dirpath.is_symlink() and not any(dirpath.iterdir()):
	dirpath.rmdir()

[greptile-apps]

Fragile pre-condition assertion

Line 133 assert not external_dir.exists() hard-codes an assumption about the test environment state. If the directory exists at test start (e.g., a previous test run crashed before cleanup, or tests are run in a non-clean environment), this assertion will fail before remove_stale_external_asset_symlinks() is even exercised, producing a misleading failure.

Consider using pytest.skip or ensuring the directory is cleaned up via a fixture:

def test_remove_stale_symlinks_no_external_dir(tmp_path, monkeypatch) -> None:
    monkeypatch.chdir(tmp_path)
    ...
    remove_stale_external_asset_symlinks()  # should not raise

Or simply skip the pre-condition check if the intent is just to verify the function handles a missing directory gracefully.

[codspeed-hq]

Merging this PR will not alter performance

✅ 8 untouched benchmarks

---

_{Comparing masenf/clean-up-old-asset-symlinks (ff962fc) with main (3c11451)}

[masenf]

@greptile-apps please re-review

[greptile-apps]

Test operates on real CWD, not an isolated temp directory

test_remove_stale_external_asset_symlinks resolves external_dir from Path.cwd() without redirecting the working directory to a tmp_path. This has two concrete risks:

1. Side-effect on real app directories: If the test suite is run from a directory that already has a real assets/external/ tree (e.g., a developer running tests inside a Reflex project), the shutil.rmtree(external_dir) in the finally block will delete all real symlinked assets, not just the test artifacts.

2. Test interaction: Because test_shared_asset (line 11) also creates and tears down assets/external/ in the same CWD, the two tests may stomp on each other if run in parallel or in certain orders.

By contrast, test_remove_stale_symlinks_no_external_dir (line 126) correctly uses monkeypatch.chdir(tmp_path). The same pattern should be applied here:

def test_remove_stale_external_asset_symlinks(tmp_path: Path, monkeypatch) -> None:
    monkeypatch.chdir(tmp_path)
    external_dir = (
        tmp_path / constants.Dirs.APP_ASSETS / constants.Dirs.EXTERNAL_APP_ASSETS
    )
    # ... rest of setup using external_dir ...
    try:
        remove_stale_external_asset_symlinks()
        ...
    finally:
        shutil.rmtree(external_dir, ignore_errors=True)

This makes the test fully hermetic regardless of the environment.

[greptile-apps]

Mutating directory while iterating its generator

external_dir.rglob("*") returns a lazy generator backed by os.scandir(). Calling path.unlink() inside the loop modifies the directory tree that the generator is still traversing. On most platforms this is harmless because already-buffered directory entries are not re-read, but it is undefined behaviour in edge cases (e.g., nested symlinks or certain network/FUSE filesystems) and can produce subtle skipping of entries.

A safer pattern is to collect all candidates first, then delete:

broken = [
    p for p in external_dir.rglob("*")
    if p.is_symlink() and not p.resolve().exists()
]
for path in broken:
    path.unlink()

This also makes the intent clearer — collect, then act.