Skip to content

fix(router): refuse route trees deeper than two levels (fs-router 0.3.2) - #287

Merged
Goosterhof merged 2 commits into
script-development:mainfrom
antonbijker-coder:fix/router-two-level-assert
Oct 8, 2026
Merged

Goosterhof merged 2 commits into
script-development:mainfrom
antonbijker-coder:fix/router-two-level-assert

Conversation

@antonbijker-coder

Copy link
Copy Markdown
Contributor

What

createRouterService now throws at construction when a child route has children of its own. The
message names the route and the two-level limit, and points at the path-composition alternative
(/parent/:parentId/child). Two-level trees are unchanged; an empty children: [] adds no route and
is accepted.

Why

fs-router's route lookup (flattenedRoutes) sees a top-level route and its direct children and nothing
below, while vue-router accepts any depth. A third-level route therefore matched, reached fs-router's
beforeEach, and threw <path> is an unknown route — at navigation time, and only on that route. The
limit was invisible until someone visited it.

Full support for deeper trees was considered and dropped: no consumer has a tree deeper than two levels
(BIO, lokalekeuze and isms use no children; town-crier, codebook and ublgenie use two), Emmie's
predecessor router has the same one-level flatten and nests in the path instead, and making
RouteName / ActualRoute recursive is the expensive part nobody needs. What was worth fixing is that
the limit was silent. (ScriptHub SH-0199.)

Design

  • Assert before createRouter. A refused tree builds no router and registers no guards.
  • Only children with entries count. children: [] contributes nothing to the lookup, so refusing
    it would reject a tree that works.
  • The route is named by name, falling back to path, so an unnamed layout route is still
    identifiable in the message.

A rewritten WR-1160 spec

components.spec.ts › should never read a route vue-router matched as a miss, at any depth (PR #281)
built a three-level tree to prove the miss test is vue-router's verdict (to.matched.length === 0),
never the flattened lookup. Construction now refuses that tree, but the invariant still matters, so the
spec is rewritten around the disagreement that remains at two levels: a parent route visited at its
own path
(/deep, whose children has no '' entry). vue-router matches the parent record; the lookup
keeps only its children. The spec asserts the hop still takes the lookup (install() rejects with
/deep is an unknown route) rather than slipping past the cancelling middleware.

Verified to discriminate: replacing the miss test with a flattened-lookup check fails this spec.

Tests

  • router.spec.ts: three-level tree throws with the full message; unnamed third-level parent is named by
    its path; children: [] does not throw; a two-level tree constructs and navigates.
  • Router package: 170/170 passing, 100% coverage, mutation score 94.18% (threshold 90) with no surviving
    mutants in the new code. oxlint, oxfmt (changed files), build and typecheck clean.

Version

fs-router 0.3.1 → 0.3.2, CHANGELOG entry added. Patch rather than minor: it closes a silent failure,
and the only trees it now refuses already failed at navigation. A minor would also cascade into
fs-auth's ^0.3.0 peer range for no consumer gain.

Docs

docs/packages/router.md gains a Two Levels Deep section with the refused shape and the
createNestedCrudRoutes alternative.

🤖 Generated with Claude Code

fs-router's route lookup sees a top-level route and its direct children
and nothing below, while vue-router accepts any depth. A third-level
route matched, reached fs-router's beforeEach, and threw "<path> is an
unknown route" at navigation time, only on that route.

createRouterService now throws at construction when a child route has
children of its own, naming the route and the two-level limit. An empty
children array adds no route and is accepted.

The WR-1160 spec that guarded "a matched route is never read as a miss"
built a three-level tree, which construction now refuses. It is rewritten
around the remaining disagreement between vue-router and the lookup: a
parent route visited at its own path.

Docs gain a "Two Levels Deep" section; fs-router 0.3.1 -> 0.3.2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@antonbijker-coder
antonbijker-coder requested a review from a team as a code owner October 7, 2026 09:16
npm audit --audit-level=high fails on two advisories published after
main's last green run: shell-quote 1.10.0 (GHSA-pqg4-j6r4-53mv,
critical; dev-only via @changesets/cli > launch-editor) and
source-map-js 1.2.1 (GHSA-68fv-2mgg-jv7q, high; via vue's compiler,
postcss and magicast). Lockfile-only: 1.12.0 and 1.2.2 are in range of
every dependent, so no manifest changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@antonbijker-coder

Copy link
Copy Markdown
Contributor Author

The first CI run went red at npm audit. Two advisories were published after main's last green run (5 Oct): shell-quote (GHSA-pqg4-j6r4-53mv, critical, dev-only) and source-map-js (GHSA-68fv-2mgg-jv7q, high). Neither is related to the router change, and main would hit the same gate. I've added a separate lockfile-only commit that bumps both within range (1.12.0 and 1.2.2) to unblock the rest of the pipeline. Happy to drop it if you'd rather fix this on main.

@Goosterhof Goosterhof added the Agent Review Requested Requesting review of specialized AI review agents. label Oct 8, 2026
@crit-ai

crit-ai commented Oct 8, 2026

Copy link
Copy Markdown
Collaborator

Crit reviewt deze pull request. Crit is een automatische reviewer. Zijn review staat als aparte review op deze pull request, onder dit bericht of, bij een mislukte ronde, na de volgende poging.

  • Issue — blokkeert de pull request. Fix het en push, of zeg in de thread wat je laat staan en waarom.
  • Nitpick — blokkeert nooit en vraagt geen antwoord. Een nitpick laten liggen is een legitieme keuze.
  • Resolven — een thread zelf resolven verandert niets. Crit leest de code en je woorden. Eerst reageren, dan pushen.
Een review lezen

Eén review per ronde. Bovenaan de telling en het oordeel. Daaronder de issues, dan de nitpicks ingeklapt.

  • Een issue op een regel die in de diff zichtbaar is, staat ook inline op die regel.
  • Een issue elders staat in de samenvatting, gemarkeerd als not on the diff, so not inline.
  • Elke nitpick eindigt met de reden waarom hij geen issue is: nitpick because ….
  • Finder trace — onder elke issue staat een ingeklapt blok met de ruwe vondst: de claim, het gevolg, de paden die de zoeker volgde en de drie tags. Voor mensen optioneel; voor een agent die de fix maakt is dit het startpunt, want hier staan de paden die de zoeker las, niet alleen de verankerde regel.
  • Still open — een eerdere thread waarvan het probleem op deze head nog staat, met de alinea die zegt wat er nog ontbreekt en dezelfde finder trace.
  • Settled, not re-filed: N — vondsten die een eerdere thread al dekt; crit post ze niet opnieuw.
Issue of nitpick

Het verschil is niet ernst. Elke vondst draagt drie tags. Geldt één nitpick-waarde, dan is het een nitpick; anders een issue. Een vaste regel kiest, geen agent.

tag waarde betekenis bucket
harm_requires nothing het gaat nu al mis, zoals de code er staat issue
harm_requires runtime_state het gaat mis onder een voorwaarde tijdens het draaien: een flag, data, timing issue
harm_requires code_change het gaat pas mis na een latere wijziging die nog niet gedaan is nitpick
harm_requires no_runtime_path geen pad bereikt het probleem: dode code, een onbereikbare tak nitpick
provenance introduced deze pull request schreef de regel issue
provenance adjacent ongewijzigde code die deze wijziging breekt of voedt issue
provenance pre_existing stond er al en deze wijziging maakt het niet erger nitpick
confidence confirmed crit volgde het pad; het klopt op deze head issue
confidence unconfirmed een echte zorg die crit niet rond kreeg; de proof gap zegt wat ontbreekt nitpick
  • Runtime is een issue. Crit noemt het mechanisme, niet een kans; hoe vaak het optreedt weeg jij.
  • Buiten de diff is geen vrijstelling. Breekt jouw wijziging bestaande code, dan is dat adjacent en een issue.
Wat doe je met een vondst

Voor een issue werken drie dingen, zolang je het zegt.

actie wat je doet wat crit doet
Fixen fix, reageer in de thread, dán push ziet de nieuwe code en sluit de thread zelf
Laten staan zeg wat je laat staan en wie het oppakt: "out of scope voor deze PR", "real, filed as KD-1341", "report aangemaakt in Kendo", "risico geaccepteerd" sluit de thread en blokkeert er niet meer op
Weerleggen leg uit waarom het niet klopt, met iets dat crit kan nakijken: een pad, een test, een meting leest de code; klopt jouw uitleg, dan sluit crit de thread

Wat niet werkt: een thread zelf resolven zonder fix of antwoord. "Werkt bij mij" en "fixen we later" tellen niet.

Een nitpick laten liggen is een legitieme keuze. Hij blokkeert niet en vraagt geen antwoord. Oppakken, een ticket maken of niets doen: alle drie prima.

oordeel wanneer
request changes minstens één issue of één open thread
approve geen van beide; nitpicks mogen blijven
comment de repo heeft blokkeren of goedkeuren uit staan
Veelgestelde vragen
Een runtime-issue komt bij ons bijna nooit voor. Mag ik hem laten staan?

Ja. Schrijf in de thread dat je het risico accepteert en waarom; crit sluit de thread. Let op: een retry, een cron-overlap of een dubbele webhook gebeurt ook bij één gebruiker.

Crit vond iets in code die ik niet heb aangeraakt. Waarom staat dat op mijn pull request?

Crit zet een vondst waar het probleem woont. Breekt jouw wijziging die code, dan is het adjacent en een issue; een oude bug los van jouw wijziging is pre_existing en een nitpick.

Wat betekent "proof gap" en wat moet ik ermee?

Crit zag een echt mechanisme maar kon één stap niet bewijzen; de proof gap zegt welke. Antwoord in de thread of dat pad bestaat, en fix het als dat zo is.

Ik heb de thread geresolved op GitHub, maar crit komt er toch weer mee. Waarom?

Crit kijkt naar de code, niet naar de knop. Staat het probleem op de nieuwe head nog, dan post crit het opnieuw; fix het of schrijf in de thread waarom je het laat staan.

Wat moet ik precies in een thread schrijven om iets te laten staan?

Noem het gedrag dat je laat staan en wie het oppakt; een Kendo-report zonder ticketnummer telt ook. Crit zoekt het report nooit op en leest alleen wat jij in de thread schrijft.

Ik heb gefixt en gepusht, maar crit ziet mijn reactie niet. Wat ging er mis?

Crit leest de threads vlak na een push, dus een reactie van daarna mist die ronde. Eerst reageren, dan pushen; de fix zelf ziet crit altijd.

Mag ik alles in de finder trace vertrouwen?

De paden en regels wel: crit liep ze na voordat de issue werd gepost. De zinnen eromheen niet altijd. De vetgedrukte kop en de alinea eronder zijn de geverifieerde tekst; waar de trace daarvan afwijkt, wint de alinea. Een absolute bijzin in de trace ("only called by tests") is een claim van de zoeker, geen oordeel.

Ik laat een agent de fix maken. Wat geef ik hem?

De hele comment, inclusief de trace. De trace kan verdere sites noemen; laat hem elke site in de repo controleren voordat hij fixt. Een issue die "three routes" zegt en er één verankert, noemt de andere twee in de trace. Een pad onder node_modules of vendor is leesspoor, geen site.

Moet ik op nitpicks reageren?

Nee. Laten liggen is een legitieme keuze: een nitpick heeft geen thread en blokkeert niet. Zolang de code hetzelfde blijft, kan hij in een volgende ronde opnieuw in de ingeklapte lijst staan.

Waarom zegt crit "request changes" terwijl er geen nieuwe issues zijn?

Een eerdere thread waarvan het probleem nog staat, blokkeert ook. Kijk onder Still open: daar staat de thread met wat er nog ontbreekt.

Kan crit per repo minder streng?

Ja, met twee schakelaars per repo: approve en request changes. Staat request changes uit, dan wordt het oordeel comment en blokkeert de pull request nooit.

@crit-ai crit-ai left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crit review

0 issues · 0 nitpicks · head 6eaed667ca

Crit approves — nothing blocking at this head.

No issues or nitpicks.

@Goosterhof
Goosterhof merged commit 1f37eb5 into script-development:main Oct 8, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Agent Review Requested Requesting review of specialized AI review agents.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants