fix: serve group roots in markdown-only - #373
Merged
NullVoxPopuli merged 1 commit intoAug 14, 2026
Merged
Conversation
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>
|
|
|
@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
marked this pull request as ready for review
August 14, 2026 22:26
NullVoxPopuli
approved these changes
Aug 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
markdown-onlyis the last test-app withouthandlePotentialIndexVisit. Its/Docsgroup 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
visitAllLinkscompares URLs. An erroring/Docsstays at/Docs, socurrent.startsWith(expected)holds and the navigation counts as successful. The only trace is a console line nothing asserts on: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/Docshere redirects without a route file and theapp/routes/page.tsadded below becomes a no-op.Keeping it anyway, because the merge order isn't fixed: on
mainas 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-apppassesskipAllLinks, 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-onlygoes from 3 tests to 4, all green:To see the bug, check out
mainand 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-descriptionandhumanizer. The author chose the scope; neither the code nor this description has been reviewed by them yet. Opened as a draft for that reason.