Skip to content

fix: serve group roots in markdown-only - #373

Merged
NullVoxPopuli merged 1 commit into
universal-ember:mainfrom
gitKrystan:gitkrystan/markdown-only-index-redirect
Aug 14, 2026
Merged

fix: serve group roots in markdown-only#373
NullVoxPopuli merged 1 commit into
universal-ember:mainfrom
gitKrystan:gitkrystan/markdown-only-index-redirect

Conversation

@gitKrystan

@gitKrystan gitKrystan commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

markdown-only is the last test-app without handlePotentialIndexVisit. Its /Docs group root errors on every run, and the suite reports green anyway.

Follow-up to #371, which found and fixed the same thing in markdown-and-gjs-md-app. I left this one alone there because it was already running in CI and the fix wasn't fallout from that rename.

Why the crawl never caught it

visitAllLinks compares URLs. An erroring /Docs stays at /Docs, so current.startsWith(expected) holds and the navigation counts as successful. The only trace is a console line nothing asserts on:

Page not found for path "/Docs". (Using group: "Docs", see console for more information)

So the new test asserts the redirect directly rather than trusting the crawl to notice.

Also here

The crawl moves to assert.deepEqual(visited.sort(), [...]), matching the other two apps after @NullVoxPopuli's review on #371. Its per-visit 250ms padding goes with it — that predates the test waiters 0d52f50 registered, and test-support 0.9.0 visits each target once. The crawl drops from 1296ms to 35ms.

Overlap with #370

#370 makes group roots resolve through routeWillChange, so once it lands /Docs here redirects without a route file and the app/routes/page.ts added below becomes a no-op.

Keeping it anyway, because the merge order isn't fixed: on main as it stands today the route is what fixes the bug, so dropping it would make this PR silently depend on #370 landing first. It stays harmless afterwards — every other test-app still wires the hook.

The rest of this PR is unaffected, and the direct redirect assertion gets more useful under #370, not less: it becomes the only thing that would catch group-root redirects regressing.

Checked and not changed

docs-app passes skipAllLinks, and I'd guessed in #371 that 0.9.0 might have made that unnecessary. It hasn't: with the flag off, the crawl hits its 120s timeout and the next three tests then fail at 180s each. Left as-is.

That leaves no test-app with a group root that errors.

Testing

markdown-only goes from 3 tests to 4, all green:

pnpm build                 # once, at the root
cd test-apps/markdown-only && pnpm build:tests && pnpm test:browser

To see the bug, check out main and watch the browser log during that run — Page not found for path "/Docs", with the suite still green.

AI attribution: 🤖 Claude using Opus 5. Used skills including pr-description and humanizer. The author chose the scope; neither the code nor this description has been reviewed by them yet. Opened as a draft for that reason.

The last test-app without `handlePotentialIndexVisit`. Its `/Docs` group
root errored on every run and the suite reported green, the same way
markdown-and-gjs-md-app did before universal-ember#371: the crawl compares URLs, and an
erroring `/Docs` keeps its own, so nothing failed. It has app/routes/page.ts
now, and a test that asserts the redirect rather than trusting the crawl to
notice.

The crawl moves to `assert.deepEqual(visited.sort(), [...])` to match the
other two apps, per @NullVoxPopuli on universal-ember#371. Its per-visit 250ms padding goes
with it: that predates the test waiters 0d52f50 registered, and test-support
0.9.0 visits each target once. 1296ms down to 35ms.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bolt-new-by-stackblitz

Copy link
Copy Markdown

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@vercel

vercel Bot commented Aug 14, 2026

Copy link
Copy Markdown

@gitKrystan is attempting to deploy a commit to the universal-ember Team on Vercel.

A member of the Team first needs to authorize it.

@gitKrystan
gitKrystan marked this pull request as ready for review August 14, 2026 22:26
@NullVoxPopuli
NullVoxPopuli merged commit 1dd8585 into universal-ember:main Aug 14, 2026
4 of 6 checks passed
@gitKrystan
gitKrystan deleted the gitkrystan/markdown-only-index-redirect branch August 14, 2026 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants