Skip to content

Add publishing routing-strategy shim to phoenix_kit_routes/0 - #524

Merged
ddon merged 2 commits into
BeamLabEU:devfrom
mdon:fix/route-collision-router-dispatch
May 8, 2026
Merged

Add publishing routing-strategy shim to phoenix_kit_routes/0#524
ddon merged 2 commits into
BeamLabEU:devfrom
mdon:fix/route-collision-router-dispatch

Conversation

@mdon

@mdonmdon commented May 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Pairs with BeamLabEU/phoenix_kit_publishing#14.

phoenix_kit_routes/0 now emits a publishing-specific dispatch shim
when PhoenixKitPublishing.RouterDispatch is loaded:

  • an internal-prefix scope at
    /<url_prefix>/__phoenix_kit_publishing_dispatch with two sub-scopes
    (/localized binding :language + :group, /root binding
    :group only), each piped through the standard :browser +
    :phoenix_kit_* pipelines plus a new :phoenix_kit_publishing_internal
    pipeline that runs RouterDispatch.restore_path/2;
  • a def call/2 override on the host router (Phoenix.Router publishes
    defoverridable init: 1, call: 2 from match_dispatch/0 — a
    documented extension point) that calls RouterDispatch.maybe_rewrite/1
    before super(conn, opts). Publishing-bound URLs get path-rewritten
    onto the internal prefix and dispatched via Phoenix's normal
    pipeline; URLs that don't resolve to a known publishing group pass
    through unchanged so host routes win Phoenix's first-match.

Compile-time gated on
Code.ensure_loaded?(PhoenixKitPublishing.RouterDispatch). Installs
that don't have publishing in the dep tree get an empty AST — no
behaviour change.

Why

Publishing's old Routes.public_routes/1 registered
/:language/:group(/*path) and /:group(/*path) directly under the
host's URL prefix. Under url_prefix: "/" those routes match every
two-or-more-segment URL, and Phoenix.Router has no per-segment regex
constraint mechanism — so any host route declared after
phoenix_kit_routes() shaped /:locale/<literal>/... was silently
shadowed: publishing's controller matched first, didn't find the
group, returned 404. Canary host's
/fr/services/view/nos-services-a-nice (and three other
/:locale/services/... routes) were dead this way after they
installed phoenix_kit_legal (which transitively pulls publishing).

The dispatch shim flips the trust direction. Publishing's catch-all
no longer sits at the host's absolute root claiming everything;
instead the override checks each request against the listing-cache /
DB and rewrites only when the URL belongs to a known publishing
group. Host routes get a fair shot at every URL.

See BeamLabEU/phoenix_kit_publishing#14 for the matching
PhoenixKitPublishing.RouterDispatch module, the policy details
(maybe_rewrite/1, restore_path/2, the defoverridable edge case
for hosts with their own def call/2), and the 19-test pinning suite.

Coordination notice

This change is conditional on PhoenixKitPublishing.RouterDispatch
being present, so:

  • Hosts running this core change with an older publishing version (no
    RouterDispatch module) — Code.ensure_loaded? returns false, the
    helper emits no-op AST, publishing's old Routes.public_routes/1
    still registers the catch-all as before. Behaviour unchanged.
  • Hosts running this core change with the new publishing version
    (RouterDispatch present, Routes.public_routes/1 returns no-op) —
    the macro emits the override + internal scope. Bug fixed.
  • Hosts running the new publishing version with an older core (no
    macro change) — publishing's Routes.public_routes/1 no longer
    registers the catch-all and the override isn't emitted, so
    publishing public URLs return Phoenix NoRouteError.
    Regression for them — so the publishing PR shouldn't ship to
    Hex without this core release also being out.

Both versions need to ship together (or core ships first, then
publishing). The publishing PR description flags the same constraint
in the other direction.

Verification

mix format --check-formatted # clean
mix credo --strict # clean (one nit fixed during the sweep)
mix test # 1055 tests, 4 failures (all pre-existing — V107
migration tests + a permissions test, unrelated
to this change; verified by stashing the diff
and re-running)

Browser smoke (workbench phoenix_kit_parent setup with the canary
collision shape — url_prefix: "/" + /:locale scope after
phoenix_kit_routes()):

URLResult
/en/services/view/test-slug200 — host's ServicesLive (was 404 pre-fix)
/fr/services/view/x200 — multi-locale
/en/db-test-1/markdown-rendering-demo200 → canonical /db-test-1/..., body shows post
/db-test-1/markdown-rendering-demo200 — canonical works directly
/ku-ku/the-new-post302 — group listing fallback (regex-collision regression closed)
/admin/dashboard302 → log-in (admin still routes)
/200 — host home
/some-unknown-route404 — genuine Phoenix NoRoute, not publishing's smart-fallback

document.documentElement.outerHTML.includes('__phoenix_kit_publishing_dispatch')
returned false on every rendered publishing page checked. Zero leakage
of the internal prefix or either discriminator segment in canonical, og,
links, JS, headers.

Canary install (separate host, four-locale config, three publishing
groups including ku-ku slug-mode) verified the same matrix
end-to-end + session/CSRF + view-source — all green after both
commits land.

Test plan

  • mix precommit clean
  • All publishing test suite (1018 tests, 0 failures) passes against the new macro shape via the matching publishing PR
  • Browser smoke covers the eight URL classes in the matrix above
  • HTML body sweep for prefix / localized / root segment leakage — 0 occurrences
  • Canary install pins both branches via git deps and re-ran the full matrix — all green
  • Pre-existing 4 test failures verified unrelated by stash-and-rerun

Files

  • lib/phoenix_kit_web/integration.ex (+94 lines: compile_publishing_routing/1 private helper + splice point in phoenix_kit_routes/0)
  • AGENTS.md (+44 lines: "Publishing routing strategy" section under "External module route discovery")

mdon added 2 commits May 9, 2026 00:13
Emit a `def call/2` override on the host router (via Phoenix.Router's
documented `defoverridable init: 1, call: 2` extension point) that
path-rewrites publishing-bound URLs onto an internal
`__phoenix_kit_publishing_dispatch` prefix. Register publishing's
catch-all under that internal scope with the standard `:browser` and
`:phoenix_kit_*` pipelines plus `RouterDispatch.restore_path/2` so
canonical-URL generation in the controller sees the URL the client
sent, not the internal prefix.
URLs that don't resolve to a known publishing group pass through
unchanged so host routes shaped `/:locale/<literal>/...` declared
after `phoenix_kit_routes()` win matching. Fixes the silent shadowing
where publishing's `/:language/:group/*path` claimed every two-or-more-
segment URL.
Compile-time gated on
`Code.ensure_loaded?(PhoenixKitPublishing.RouterDispatch)`; installs
without publishing in the dep tree get an empty AST. `apply/3` is
used to dodge the static-resolution warning when the optional dep is
absent.
Pair commit with `phoenix_kit_publishing` 797e40d, which adds the
`localized_segment/0` and `root_segment/0` helpers on
`PhoenixKitPublishing.RouterDispatch`. The `compile_publishing_routing/1`
helper now splits the internal-prefix scope in two:
* `/<prefix>/__phoenix_kit_publishing_dispatch/localized/:language/:group(/*path)`
* `/<prefix>/__phoenix_kit_publishing_dispatch/root/:group(/*path)`
The override emitted in the host router calls
`RouterDispatch.maybe_rewrite/1`, which now returns rewrites that
include the appropriate discriminator segment. Phoenix's matcher sees
only one viable route shape per sub-scope and dispatches the
controller with correct bindings (no more `language=<group-slug>`
collision when the slug matches the language-code regex).
AGENTS.md updated to describe the sub-scope split.
Verified: original collision fix still holds (host route wins for
`/<locale>/services/view/<slug>`); the no-locale slug-mode regression
the canary install caught is closed.
@ddon
ddon merged commit 9a7fbad into BeamLabEU:devMay 8, 2026
ddon added a commit that referenced this pull request May 8, 2026
…525
Code/doc fixes addressing one finding per PR (or several where
trivial). Each closes a NITPICK or IMPROVEMENT-LOW from the
matching CLAUDE_REVIEW.md; design-level / breaking / risky items
deferred per the FOLLOW_UP.md "Skipped" sections.
- #516: Drop dead `String.to_atom` fallback in OAuth interpolate_url
- #518: Delete stray 0-byte pages_html.ex
- #519: Fix stale `viewer={true}` template comment + login_path
trailing-slash self-loop guard
- #521: Resolution-order doc on permission_key_for_admin_view/1
- #522: Hot-reload safety pitfall in per-module-i18n.md
- #523: KnownPackages — max-pages cap, ensure_table race comment,
Logger-levels operational signals in moduledoc
- #524: __mix_recompile__?/0 note next to apply/3 explanation
- #525: LanguageSwitcher attr doc atom/string keys + DRY resolve_url
per-language + JS sortable:flash defensive status check
Plus FOLLOW_UP.md per PR enumerating closed vs deferred items.
PR #525's FOLLOW_UP also captures the bundled DnD audit trail
(table_default drag-handle scoping, sortable:flash, TR cell-width
preservation) absent from the original PR body.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ddon added a commit that referenced this pull request May 8, 2026
Three-day window of accumulated work since 1.7.105 (2026-05-05):
PRs #516, #518, #519, #521, #522, #523, #524, #525, plus the
review-doc suite and post-merge triage closing nitpicks across all
eight.
Headline changes — V111 PDF library tables, DB module extracted to
phoenix_kit_db, MediaBrowser modal viewer, sidebar gettext API, live
Hex.pm catalog, publishing routing-strategy shim closing the
/:locale/<literal>/... host-route shadowing bug, LanguageSwitcher
:per_translation_urls, and bundled DnD improvements (drag-handle
scoping, sortable:flash, TR cell-width preservation).
All changes are strictly additive / non-breaking; one transitional
extraction (DB → phoenix_kit_db) requires the paired Hex package
once it ships.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@mdon@ddon