src/tracing.ts (1)

91-100: Mutating route.data is idempotent but shared — confirm intent.

route.data.handler = wrapEventHandler(...) and the middleware remap mutate the returned route's data object. If ~findRoute returns a reference to an entry in h3["~routes"] (the typical radix router behavior), the handler/middleware are replaced in-place. This is safe here because wrapEventHandler/wrapMiddleware are idempotent via __traced__, and it mirrors the existing h3.on wrapping (Line 121-122). Worth a one-line comment noting the intentional mutation so a future refactor doesn’t accidentally clone the route and break the idempotence on subsequent requests.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tracing.ts` around lines 91 - 100, Add a one-line explanatory comment
inside the wrappedFindRoute function above the mutations to route.data (where
route.data.handler is set with wrapEventHandler and route.data.middleware is
remapped with wrapMiddleware) stating that this in-place mutation is intentional
because findRoute returns shared references (e.g., h3["~routes"]) and the
wrappers are idempotent via __traced__; mention it mirrors the existing h3.on
wrapping behavior (originalFindRoute / h3.on) and warn future maintainers not to
clone the route object or change to non-idempotent wrappers.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

test/tracing.test.ts (2)

1206-1248: Strong test for the double-wrapping guard.

Exactly-3 assertion on routeAsyncStarts is the right shape — it pins the __traced__ short-circuit in wrapEventHandler (src/tracing.ts:61-75) against the memoized-route case from frameworks like nitro.

Two minor notes:

1. Because wrappedFindRoute mutates route.data.handler in place (src/tracing.ts context snippet 1), after the first request cachedRoute.data.handler is replaced with the traced wrapper. An additional assertion that cachedRoute.data.handler.__traced__ === true and that it's the same reference across runs would make the non-double-wrap contract explicit rather than inferred from the count.
2. If the wrapper ever regresses to returning a new wrapper each call, the count would still be 3 (one per request) — the current assertion wouldn't catch handler re-wrapping per se, only per-request duplication. Consider also spying on wrapEventHandler behavior indirectly by asserting handler identity stability:

Optional stronger assertion

     await run();
     await run();
     await run();
+
+    // Handler should be wrapped exactly once and reused across calls.
+    expect((cachedRoute.data.handler as any).__traced__).toBe(true);
 
     await new Promise((resolve) => setTimeout(resolve, 10));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/tracing.test.ts` around lines 1206 - 1248, Add explicit assertions that
the memoized route handler is mutated to the traced wrapper and remains the same
reference across runs: after the first run assert
cachedRoute.data.handler.__traced__ === true and save a reference (e.g., const
firstHandler = cachedRoute.data.handler) then after subsequent runs assert
cachedRoute.data.handler === firstHandler; this verifies
wrapEventHandler/wrappedFindRoute actually short-circuits re-wrapping rather
than relying solely on the asyncStart count. Use the existing cachedRoute,
tracingPlugin, and wrapEventHandler/wrappedFindRoute behavior to locate where to
add these checks in the test.

---

1160-1204: LGTM — good coverage for nitro-style ~findRoute.

Test correctly verifies route tracing works when handlers are only exposed via a custom ~findRoute (not pushed into ~routes), which is exactly the regression from nitrojs/nitro#4211.

One small optional tightening: the test only asserts routeEvents.length > 0 but doesn't verify the handler actually ran / returned its value. Adding an assertion on app.handler(event)'s resolved value (or on routeEvents[0].asyncStart?.data.event.url.pathname === "/file-route") would make a regression where the wrapper swallows the handler result more obvious.

Also, the comment on Line 1187-1188 referring to "the order documented in the suggested fix" reads as review-context leakage — consider rephrasing to something self-contained like "Apply tracing plugin after ~findRoute is set to verify the getter/setter wrapper picks up the reassigned function."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/tracing.test.ts` around lines 1160 - 1204, Add a concrete assertion that
the handler actually executed and returned the expected value and make the
comment self-contained: after awaiting app.handler(event) capture its resolved
result and assert it equals "file-based response" (or assert
routeEvents[0].asyncStart?.data.event.url.pathname === "/file-route") to ensure
the wrapper didn't swallow the return value; also change the comment referencing
"the order documented in the suggested fix" to a self-contained sentence like
"Apply tracing plugin after ~findRoute is set to verify the getter/setter
wrapper picks up the reassigned function." Reference H3Core, tracingPlugin,
app["~findRoute"], H3Event, and routeEvents in the test to locate the changes.

src/tracing.ts (1)

98-100: Optional: skip reallocating route.data.middleware when all entries are already traced.

On every request the wrapper unconditionally allocates a fresh array via .map(wrapMiddleware), even when route.data is a cached object whose middleware entries are already __traced__. wrapMiddleware is a no-op in that case, but the array allocation/assignment still runs per request. Minor and safe to defer.

♻️ Proposed micro-optimization

-        if (route?.data.middleware) {
-          route.data.middleware = route.data.middleware.map((m) => wrapMiddleware(m));
+        if (route?.data.middleware && route.data.middleware.some((m) => !(m as MaybeTracedMiddleware).__traced__)) {
+          route.data.middleware = route.data.middleware.map((m) => wrapMiddleware(m));
         }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/tracing.ts` around lines 98 - 100, The code currently always reassigns
route.data.middleware = route.data.middleware.map((m) => wrapMiddleware(m));
avoid allocating a new array when all middleware are already traced by checking
the __traced__ flag first: inspect route.data.middleware (use
Array.prototype.some or every) to detect any entry without m.__traced__, and
only call .map(wrapMiddleware) and reassign when at least one middleware is not
traced; leave route.data.middleware untouched otherwise. Ensure you reference
route.data.middleware, wrapMiddleware, and the __traced__ property in your
change.