ENG-9917 fix: run plugin post_compile hooks at most once per app instance - #6733
ENG-9917 fix: run plugin post_compile hooks at most once per app instance#6733FarhanAliRaza wants to merge 4 commits into
Conversation
App.__call__ runs whenever the ASGI app is constructed, and it re-ran every plugin's post_compile each time. post_compile hooks mutate the app (add_middleware, mounting routes), so a second construction on the same app instance (a test harness rebuilding the ASGI app) re-applied those mutations — e.g. a middleware installed twice gates every event twice. Latch the hook loop per app instance, mirroring the once-per-process guarantees elsewhere in the compile path. The latch is set after the loop completes so a failed hook is retried on the next construction rather than silently skipped. The guarantee is documented on Plugin.post_compile so plugins don't need their own idempotency guards. Fixes ENG-9917.
Greptile SummaryThis PR changes plugin post-compile hooks to run once per app instance. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "fix: track completed post_compile hooks ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Merging this PR will not alter performance
Comparing 
Footnotes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 337341739e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not self._plugins_post_compiled: | ||
| for plugin in config.plugins: | ||
| plugin.post_compile(app=self) | ||
| self._plugins_post_compiled = True |
There was a problem hiding this comment.
Because the latch is only set after the whole plugin loop succeeds, a retry after a later plugin raises will start from the first plugin again. If an earlier plugin already mutated the app (for example by adding middleware or routes) and a subsequent plugin fails once, the next app() call duplicates the earlier mutation, so the new at-most-once guarantee still fails in the explicit retry-after-failure path.
Useful? React with 👍 / 👎.
Gating only the plugin post_compile loop left the same repeat-mutation bug elsewhere in App.__call__: the compiled-frontend mount was appended to the persistent self._api on every construction, and a Starlette api_transformer was re-mounted with an extra CORS layer each call. A single boolean latch also re-ran already-succeeded hooks when a later plugin's hook raised, reinstalling their middleware on retry. Extract the assembly steps (plugin hooks, frontend mount, transformer wiring, top-level app) into _assemble_asgi_app, cache its result per instance, and track completed hooks per plugin so a failed hook is retried without re-running the ones that already mutated the app. _compile intentionally still runs on every call: forked prod workers re-enter __call__ to re-evaluate stateful pages backend-side.
FarhanAliRaza
changed the title
| # Assembly runs at most once per app instance; _compile stays per-call. | ||
| if self._cached_asgi_app is None: |
There was a problem hiding this comment.
app() call, this gate returns the cached ASGI app and never enters _assemble_asgi_app() again. If get_config().plugins is changed on the same App instance between ASGI constructions, the new plugin is never inspected, so its post_compile hook cannot add its middleware or routes. The per-plugin set only helps when assembly runs again; this cache blocks that path entirely.
masenf
left a comment
masenf
left a comment
There was a problem hiding this comment.
It's potentially problematic that we allow a recompile on the same instance, but don't re-run the hooks on the same instance.
If the second compile adds a page, and a plugin's post_compile does something with pages, then the newly added page will be compiled into the app, but won't be affected by the plugin.
Is there a legit use case where this hits? or does it just happen in testing?
My thought would be if we run _compile, then we should run post_compile hooks.
Maybe instead we just need to reset things on the app instance, like the pages/routes, middleware, lifespan tasks, etc to make it safe to re-__call__ the app instance and essentially rebuild it. (I think properly speaking we should be attaching these to the returned asgi and maybe not mutating the App at all, but that can come later)
Real plugins are dataclasses (eq=True sets __hash__ = None), so storing them in a set raised TypeError: unhashable type on every app that uses the default TailwindV4Plugin. Key the completed-hook record by id() instead, keeping the plugin as the value so its id cannot be recycled; identity is also the right semantics, since equal-but-distinct plugin instances must each run their hook. Also add the reflex-base news fragment required by the per-package changelog check for the Plugin.post_compile docstring change.
2 participants
Fixes ENG-9917 at the framework level.
Problem
App.__call__runs every plugin'spost_compile(app=self)each time the ASGI app is constructed.post_compilehooks mutate the app —add_middleware, mounting routes — so constructing the ASGI app twice on the same app instance (a test harness rebuilding it, repeatedapp()calls) re-applies those mutations. The concrete instance that surfaced this: enterpriseAuthPlugininstalling a secondAuthMiddleware, gating every event twice. But the bug class belongs to the lifecycle, not to any one plugin — every app-mutatingpost_compilehook is affected.Fix
Latch the hook loop with a per-app-instance flag (
_plugins_post_compiled), sopost_compileruns at most once per app instance. The flag is set after the loop completes, so a hook that raises (e.g. a plugin rejecting a misconfigured app) is retried on the next construction instead of being silently skipped. The at-most-once guarantee is documented onPlugin.post_compileso plugin authors don't hand-roll idempotency guards.Backend-only processes (
REFLEX_SKIP_COMPILE) are unaffected: they construct a freshAppper process, and the flag starts false. Dev hot reload restarts the worker process (freshApp), so hooks still re-run there as before.Tests
test_call_app_runs_plugin_post_compile_once— regression test, failed before the fix (hook ran twice).test_call_app_retries_post_compile_after_failure— pins the not-latched-on-failure semantics.Full unit suite passes (6619, coverage 77.97%), ruff + pyright clean.