refactor: Middleware to use chi's Use instead of custom plug chain

[ainsleyclark]

Summary

This PR refactors the middleware system to leverage chi's built-in Use method for global middleware instead of maintaining a separate plugs slice. This simplifies the codebase and ensures middleware runs before route matching, making it suitable for cross-cutting concerns like CORS and logging.

Key Changes

• Removed plugs field from the Kit struct and its initialization
• Refactored Plug method to directly register middleware with chi's Use instead of appending to a slice
• Removed plug chain execution from the handle method since middleware now runs at the chi router level
• Simplified Group method by removing the logic that manually applied parent plugs to sub-routers (no longer needed since plugs are registered globally on chi)
• Enhanced test coverage for Plug with two test cases:
  • Verifies context values set by middleware are visible to handlers
  • Verifies middleware runs on OPTIONS preflight requests to GET-only routes

Implementation Details

The Plug method now wraps each middleware function to:

1. Create a new context from the request/response
2. Execute the plug with a next handler that updates the request and calls the chi handler
3. Handle any errors through the kit's error handler

This approach ensures middleware executes before route matching at the chi level, providing the expected behavior for global middleware while maintaining compatibility with the existing Handler and Plug function signatures.

https://claude.ai/code/session_01Y1KRWLLr2YC6Sisy2w2ZAJ

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[claude]

Review summary

• Overall score: 7/10
• Critical issues: 0
• Warnings: 3
• Suggestions: 2
• Recommendation: ⚠️ Approve with comments

The refactor successfully simplifies the middleware plumbing by delegating to chi's Use. Context propagation is correct (middleware sets ctx.Request via ctx.Set, which is threaded into the next chi handler via r = c.Request), and the ordering of multiple plugs is preserved. The main gap is test coverage for multi-plug scenarios and the group + global middleware combination.

---

Critical issues 🔴

None

---

Warnings 🟡

1. No test for multiple Plug registrations

The existing tests only exercise a single plug. With the new loop-based k.mux.Use approach, ordering and correct per-plug capture should be verified.

// Missing: register two plugs and assert both ran in order
app.Plug(plugA, plugB)

2. No test that Group routes inherit parent Plug middleware

The old Group method explicitly copied k.plugs onto the sub-router. The new code relies silently on chi's mount semantics (parent Use middleware runs before the mounted sub-router). A regression test would pin this behaviour:

app.Plug(globalMiddleware)
app.Group("/api", func(sub *Kit) {
    sub.Get("/ping", handler)
})
// assert globalMiddleware ran for GET /api/ping

3. Unrelated files changed without explanation

internal/appdef/jsonformat/formatter.go and pkg/cache/mem.go both change reflect.Ptr → reflect.Pointer. These are valid (reflect.Ptr is deprecated since Go 1.17), but they are out of scope for a middleware refactor and could be a separate commit to keep the diff focused.

---

Suggestions 🟢

1. Document the context-propagation contract in the Plug doc comment

The middleware creates its own *Context (separate from the route handler's). Values survive into the route handler only because r = c.Request threads the updated *http.Request through chi's chain. A one-liner in the doc comment would prevent future confusion:

// Values stored via ctx.Set in a Plug are propagated to the route handler
// through the request context; they are visible via ctx.Get in the handler.

2. Evaluate whether plug variable naming shadows the Plug type in the loop

for _, plug := range plugs — plug (variable) shadows Plug (type) within the loop body. This is harmless in Go but can confuse readers. mw or p are clearer alternatives.