Allow plugins to decorate an existing core route (route overrides) - #2961
Open
malinthaprasan wants to merge 3 commits into
Open
Allow plugins to decorate an existing core route (route overrides)#2961malinthaprasan wants to merge 3 commits into
malinthaprasan wants to merge 3 commits into
Conversation
malinthaprasan
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Induwara04,
Krishanx92,
PasanT9,
Piumal1999,
RakhithaRR,
Tharsanan1,
Thushani-Jayasekera,
VirajSalaka,
ashera96,
chamilaadhi,
dushaniw,
hisanhunais,
lasanthaS,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika,
tharikaGitHub,
tharindu1st and
thivindu
as code owners
July 28, 2026 14:35
Contributor
📝 WalkthroughWalkthroughChangesThe PR adds a recorder-based router abstraction, updates core handlers to register through it, and introduces plugin route overrides. Plugins can decorate or replace core handlers, with validation, response capture utilities, deferred installation, and startup error handling. Core routing and plugin route overrides
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
malinthaprasan
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@platform-api/pdk/override.go`:
- Around line 139-160: Update WriteCaptured to clear each destination header key
before adding its captured values, ensuring captured headers replace existing
upstream values rather than append duplicates. Preserve the existing
Content-Length exclusion and status/body handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 047bb071-e076-4a83-819e-fe588316c3f2
📒 Files selected for processing (31)
platform-api/internal/handler/api.goplatform-api/internal/handler/api_deployment.goplatform-api/internal/handler/api_key.goplatform-api/internal/handler/apikey_user.goplatform-api/internal/handler/application.goplatform-api/internal/handler/auth_login.goplatform-api/internal/handler/gateway.goplatform-api/internal/handler/gateway_internal.goplatform-api/internal/handler/llm.goplatform-api/internal/handler/llm_apikey.goplatform-api/internal/handler/llm_deployment.goplatform-api/internal/handler/llm_proxy_apikey.goplatform-api/internal/handler/mcp.goplatform-api/internal/handler/mcp_deployment.goplatform-api/internal/handler/organization.goplatform-api/internal/handler/project.goplatform-api/internal/handler/secret.goplatform-api/internal/handler/subscription_handler.goplatform-api/internal/handler/subscription_plan_handler.goplatform-api/internal/handler/websocket.goplatform-api/internal/plugin/plugin.goplatform-api/internal/router/router.goplatform-api/internal/router/router_test.goplatform-api/internal/server/external_plugin.goplatform-api/internal/server/overrides.goplatform-api/internal/server/overrides_test.goplatform-api/internal/server/plugins.goplatform-api/internal/server/server.goplatform-api/pdk/override.goplatform-api/pdk/override_test.goplatform-api/platform/override.go
Comment on lines
+139
to
+160
| func WriteCaptured(w http.ResponseWriter, res *CapturedResponse) { | ||
| if res == nil { | ||
| return | ||
| } | ||
| dst := w.Header() | ||
| for k, vs := range res.Header { | ||
| if http.CanonicalHeaderKey(k) == "Content-Length" { | ||
| continue | ||
| } | ||
| for _, v := range vs { | ||
| dst.Add(k, v) | ||
| } | ||
| } | ||
| status := res.Status | ||
| if status == 0 { | ||
| status = http.StatusOK | ||
| } | ||
| w.WriteHeader(status) | ||
| if len(res.Body) > 0 { | ||
| _, _ = w.Write(res.Body) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
git ls-files | grep -F 'platform-api/pdk/override.go' || true
echo "== file outline =="
if [ -f platform-api/pdk/override.go ]; then
ast-grep outline platform-api/pdk/override.go || true
echo "== relevant lines =="
cat -n platform-api/pdk/override.go | sed -n '1,240p'
fi
echo "== tests for WriteCaptured/captureWriter =="
rg -n "WriteCaptured|captureWriter|CapturedResponse|Content-Type" platform-api/pdk -S || true
echo "== static verifier of header merging semantics in Go stdlib =="
cat > /tmp/header_check.go <<'EOF'
package main
import (
"net/http"
"fmt"
)
func main() {
dst := http.Header{}
dst.Set("Content-Type", "old")
for k := range dst {
dst.Del(k)
}
dst.Add("Content-Type", "new")
fmt.Println(dst.Get("Content-Type"), dst["Content-Type"])
}
EOF
go run /tmp/header_check.goRepository: wso2/api-platform
Length of output: 10349
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant tests =="
cat -n platform-api/pdk/override_test.go | sed -n '1,180p'
echo "== writeCaptured call sites =="
rg -n "WriteCaptured\\(" platform-api -S
echo "== programmatic evidence: Go Add preserves existing header while Get returns first value =="
cat > /tmp/header_merge.go <<'EOF'
package main
import (
"fmt"
"net/http"
)
func main() {
dst := http.Header{}
dst.Set("Content-Type", "old")
dst.Add("Content-Type", "new")
fmt.Printf("Get=%q\n", dst.Get("Content-Type"))
fmt.Printf("Values=%v\n", dst["Content-Type"])
}
EOF
go run /tmp/header_merge.goRepository: wso2/api-platform
Length of output: 7040
Clear destination headers before adding captured headers.
dst.Add(k, v) appends captured header values to any existing values for the same key, so clients can receive duplicate headers instead of the captured response overriding upstream defaults. Clear the key before adding each captured header value.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@platform-api/pdk/override.go` around lines 139 - 160, Update WriteCaptured to
clear each destination header key before adding its captured values, ensuring
captured headers replace existing upstream values rather than append duplicates.
Preserve the existing Content-Length exclusion and status/body handling.
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
1 participant
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.
Purpose
A wrapper embedding platform-api as a library can add routes and chain middleware, but cannot change what an existing core endpoint returns. The only options are forking the handler or shadowing the route, and
ServeMuxpanics on a duplicate pattern.Goals
Let a plugin decorate one existing core route: its
Wrapreceives the original core handler asnext. Declared explicitly (so it is auditable), fails startup on a pattern core doesn't register, and leaves the route's scopes untouched.Approach
Three commits, each buildable on its own:
internal/router— aRouterinterface (*http.ServeMuxsatisfies it) and aRecorderthat records registrations instead of serving them. Core handlers'RegisterRoutesnow takerouter.Router; thepdk.Plugin/plugin.Plugincontracts keep*http.ServeMux, so no plugin or wrapper changes.pdk/override.go—RouteOverride,RouteOverrideProvider, and the capture helpersInvoke/WriteCaptured, re-exported fromplatform.installCoreRoutesvalidates them and installs each recorded route on the mux, wrapped where claimed.Registering under the same pattern is what keeps
r.PathValuecorrect inside the core handler. A nested mux behind a"/"catch-all was rejected: it costs plugin routes their405, and makesnextthe whole mux, so a decorator rewritingr.URL.Pathwould re-dispatch the request to a different endpoint after auth and scope enforcement had already run against the original path.Constraints, documented rather than worked around: an override never changes the route's scopes; one plugin per route; patterns match as exact strings; streaming routes can't be overridden (the capture writer is deliberately neither a
Flushernor aHijacker). A plugin route colliding with a core pattern is now a startup error naming the pattern instead of aServeMuxpanic.User stories
Documentation
N/A — extension surface only, no user-facing API change. The contract is documented in the doc comments on
pdk.RouteOverride,Invoke, andWriteCaptured.Automation tests
internal/router100.0% statement coverage,pdk96.4%,installCoreRoutes100.0%. Cover ordering and deferred errors in the recorder, capture/write semantics, and every startup-failure path (unknown pattern, duplicate claims, nilWrap, empty pattern, plugin/core collision).405preserved, and a bad pattern refusing startup.Security checks
go vetclean.An override cannot widen access. Required scopes are keyed by OpenAPI path/method and are untouched, so a decorated route keeps the requirement it had. A decorator cannot re-route a request either — the handler for the pattern is already selected before
Wrapruns, so rewriting the path changes nothing about what executes. The contract documents that a decorator must read the organization from request context, never from request input. Every malformed or unmatched override aborts startup rather than being skipped.Samples
A decorator that only observes the outcome should not use
Invokeat all — wrap thehttp.ResponseWriterto record the status instead. Buffering a whole response to read a status code costs memory for nothing, andInvoke's writer is deliberately not aFlusher/Hijacker.Related PRs
None.
Test environment
Go 1.26.5, macOS.
go build ./...,go build ./cmd/main.go,go vet, and the test suite all clean; each of the three commits was checked out and built independently.