[AdamMagued]

Resolves #4844.

[wahajahmed010]

Good catch on the trailing-slash-vs-param-route interaction — that bug has been in the tree for a while and the regression tests cover both directions plus the TSR fallback case (3 and 4) which is what would have caught this originally.

Substantive concerns, mostly on the diff shape:

1. cTsr is computed but never reset. The new pattern is "compute cTsr at the rollback site, OR it onto value.tsr on the way out." But value.tsr is sticky across the entire walk (it's set/cleared on value, not on the node). In the new path == prefix branch you correctly do value.tsr = false when a handler is found, but in the early !n.wildChild branch you do not. Walking a tree like /a + /:p where /a/ is also registered:

• start, path="/a", descend
• roll back case fires cTsr = path == "/" && n.handlers != nil — false here, OK
• but the prior path through the loop already set value.tsr from a cTsr on an earlier backtrack before this rollback was reached
• on return the final value.tsr = value.tsr || cTsr can OR in a stale boolean from a branch we already abandoned

I'd add value.tsr = false at every handler-found return (you do this in two of the three new ones, but not in the n.handlers != nil branch of the second case block in the diff) and also at the top of walk: so each call starts clean. Or document explicitly why the sticky-OR is safe — but I don't think it is in every case this diff currently sets it up.

2. n.children[0] access on a possibly-empty tree. In the rewritten notFound branch you do n = n.children[0] and then cTsr = (n.path == "/" && n.handlers != nil) || (n.path == "" && n.indices == "/"). If n.children is empty (degenerate case — possible after manual tree mutation in tests), this panics. The previous code guarded with if len(n.children) == 1, and you kept that guard, good — but n was assigned before the guard in the original and after in the new code. Wait, re-reading: you kept if len(n.children) == 1 { n = n.children[0]; ...}. Fine, but cTsr is initialized to false before that block, so the loop fallback won't run if the guard fails — which matches old behavior. OK.

3. Dead code I want to call out. The diff drops:

if path == "/" && n.wildChild && n.nType != root { value.tsr = true; return value }
if path == "/" && n.nType == static { value.tsr = true; return value }

and the equivalent at the end. They're now folded into cTsr = (path == "/" && n.wildChild && n.nType != root) || (path == "/" && n.nType == static). Functionally equivalent on the success path, but the old code returned immediately; the new one keeps going through the rollback loop. That's a behavior change: previously value.fullPath would not be set in those branches (the caller relied on tsr == true to drive a redirect). Confirm the redirect logic in RedirectTrailingSlash (context.go / gin.go) doesn't depend on fullPath being empty in this case — I think it doesn't, but a quick double-check on the call site would be good.

4. Test coverage. Cases 1, 2, 3, 4 are good. Add one: /{a}/b/ + /{a}/b with a request to /{actual}/b — wildcard sibling interaction, which is a slightly different failing mode than the literal-vs-param case you've covered.

5. Minor: go.mod bumps quic-go from 0.60.0 → 0.63.0. Belongs in its own dep bump PR. Easy to revert out of this one.

[AdamMagued]

Addressed review feedback:

1. Dependency Management: Reverted quic-go from v0.63.0 back to v0.60.0 in go.mod and go.sum.
2. Sticky TSR State: Reset value.tsr = false at getValue entry and ensure clean returns on matched handlers. Exits without matching handlers preserve TSR recommendations via value.tsr = value.tsr || cTsr.
3. CatchAll Guard: Added len(n.children) > 0 guard prior to accessing n.children[0] in catchAll TSR validation.
4. Param Walk Hygiene: Removed improper backtracking loops within case param that could misidentify wildcard suffixes, restoring correct precedence for wildcard sibling routing.
5. RedirectTrailingSlash: Confirmed that trailing slash redirection operates on c.Request.URL.Path and does not rely on fullPath.
6. Test Coverage: Added test cases covering wildcard sibling interaction (/:a/b/ and /:a/b with requests to /foo/b and /foo/b/) across both registration orders in tree_test.go and gin_test.go.

Full test suite passes with race detection enabled.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.65%. Comparing base (0f09c3a) to head (a010002).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #4889   +/-   ##
=======================================
  Coverage   98.65%   98.65%           
=======================================
  Files          48       48           
  Lines        3188     3198   +10     
=======================================
+ Hits         3145     3155   +10     
  Misses         43       43           

Flag	Coverage Δ
--ldflags="-checklinkname=0" -tags sonic	98.64% <100.00%> (+<0.01%)	⬆️
-tags go_json	98.58% <100.00%> (+<0.01%)	⬆️
-tags nomsgpack	98.63% <100.00%> (+<0.01%)	⬆️
go-1.26	98.34% <100.00%> (+<0.01%)	⬆️
go-1.27	98.67% <100.00%> (+<0.01%)	⬆️
macos-latest	98.65% <100.00%> (+<0.01%)	⬆️
ubuntu-latest	98.65% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.