Alanxtl commented on code in PR #921:
URL: https://github.com/apache/dubbo-go-pixiu/pull/921#discussion_r3198543066
##########
skills/pixiu-filter-author/references/filter-interface.md:
##########
@@ -0,0 +1,144 @@
+# The Four (Plus Two) HTTP Filter Interfaces
+
+Source of truth: `pkg/common/extension/filter/filter.go`. Read that file
+first; this reference exists to annotate the shape and the "why".
+
+## Interfaces, in implementation order
+
+### 1. `HttpFilterPlugin`
+
+```go
+type HttpFilterPlugin interface {
+ Kind() string // unique identifier, e.g.
"dgp.filter.http.cors"
+ CreateFilterFactory() (HttpFilterFactory, error)
+}
+```
+
+Every package has exactly one `Plugin` struct that implements this. It
+is what gets registered in `init()` via `filter.RegisterHttpFilter`.
+`Kind()` returns a package-level `const Kind = "..."` — define it once
+at the top of the file and reuse everywhere.
+
+### 2. `HttpFilterFactory`
+
+```go
+type HttpFilterFactory interface {
+ Config() any
+ Apply() error
+ PrepareFilterChain(ctx *http.HttpContext, chain FilterChain) error
+}
+```
+
+Factories are **listener-scoped singletons** (one per listener into
+which the filter is wired). `Config()` returns a pointer so that
+pixiu's config manager can deserialize yaml into it. `Apply()` runs
+**after** deserialization — this is where you validate fields and set
+defaults. `PrepareFilterChain` is called **per request**: construct a
+fresh `Filter` instance, append it to the chain.
+
+Two correctness rules that bit many contributors:
+
+- `Config()` must return a **pointer**, not the struct by value. Yaml
+ binding silently no-ops on a non-pointer.
+- Do not share `factory.cfg` pointer with the per-request `Filter`.
+ Copy out the fields you need so a hot-reload of the factory config
+ does not race with in-flight requests.
+
+### 3. `HttpDecodeFilter`
+
+```go
+type HttpDecodeFilter interface {
+ Decode(ctx *http.HttpContext) FilterStatus
+}
+```
+
+Invoked in **declaration order** as the request travels inbound. Return
+`filter.Continue` to move on or `filter.Stop` to short-circuit. Common
+decode work: authentication, rate limiting, path rewriting, enriching
+`ctx.Params`, emitting request-size metrics.
+
+### 4. `HttpEncodeFilter`
+
+```go
+type HttpEncodeFilter interface {
+ Encode(ctx *http.HttpContext) FilterStatus
+}
+```
+
+Invoked in **reverse declaration order** on the response path. Common
+encode work: response header rewriting, body transformation, masking
+PII, emitting response-size metrics.
+
+A single filter package may implement either, neither (rare), or both.
+If both, `PrepareFilterChain` calls both `chain.AppendDecodeFilters`
+and `chain.AppendEncodeFilters`.
+
+### 5. `NetworkFilterPlugin` (L4)
Review Comment:
这个L4是啥
##########
skills/pixiu-filter-author/references/pluginregistry-howto.md:
##########
@@ -0,0 +1,92 @@
+# `pkg/pluginregistry/registry.go` — The Single Boot Activator
+
+This file exists because Go's `init()` only runs for packages that are
+**imported somewhere in the build graph**. Filters register themselves
+in `init()`, so unless something imports their package, the `init()`
+never runs and the registration never happens. `pluginregistry` is the
+one file that imports every filter/adapter/listener/lb package for side
+effects — a single central blank-import list — and `cmd/pixiu/` imports
+`pluginregistry`.
+
+## Shape of the file
+
+```go
+package pluginregistry
+
+import (
+ _ "github.com/apache/dubbo-go-pixiu/pkg/adapter/dubboregistry"
+ _ "github.com/apache/dubbo-go-pixiu/pkg/adapter/llmregistry"
+ _ "github.com/apache/dubbo-go-pixiu/pkg/adapter/mcpserver"
+ // ...
+ _ "github.com/apache/dubbo-go-pixiu/pkg/cluster/loadbalancer/maglev"
+ // ...
+ _ "github.com/apache/dubbo-go-pixiu/pkg/filter/accesslog"
+ _ "github.com/apache/dubbo-go-pixiu/pkg/filter/cors"
+ // ...
+ _ "github.com/apache/dubbo-go-pixiu/pkg/filter/http/dubboproxy"
+ _ "github.com/apache/dubbo-go-pixiu/pkg/filter/http/httpproxy"
+ // ...
+ _ "github.com/apache/dubbo-go-pixiu/pkg/listener/http"
+ // ...
+)
+```
+
+All imports are in one block. Entries are **alphabetical within each
+logical group** — adapters, lb, retry, filter (top-level),
+filter/http/, filter/network/, filter/auth/, listener. When adding,
+keep the alphabetical order *within* the group your package belongs to.
+
+## How to add an import
+
+For a new filter at `pkg/filter/mynewfilter/`:
+
+```go
+_ "github.com/apache/dubbo-go-pixiu/pkg/filter/mynewfilter"
+```
+
+Placement: in the top-level `pkg/filter/` group, alphabetically between
+`_ "github.com/apache/dubbo-go-pixiu/pkg/filter/metric"` and
+`_ "github.com/apache/dubbo-go-pixiu/pkg/filter/opa"` (or wherever
+`mynewfilter` sorts).
+
+For a new proxy-style filter at `pkg/filter/http/mynewproxy/`:
+
+```go
+_ "github.com/apache/dubbo-go-pixiu/pkg/filter/http/mynewproxy"
+```
+
+Placement: in the `pkg/filter/http/` group, alphabetically.
+
+## What happens if you forget
+
+- `go build ./...` — passes.
+- `go test ./pkg/filter/mynewfilter/...` — passes.
+- `./dubbo-go-pixiu gateway start -c configs/conf.yaml` — **fails at
+ config load** with `no filter found for name dgp.filter.http.mynew`.
+
+That boot-time error is the earliest signal. There is no compile-time
+check because the blank import is literally an import for side effects
+only.
+
+## Automating the check
+
+`scripts/gen-registry-import.sh` (in this skill) walks `pkg/filter/` and
+`pkg/filter/http/`, grep's each package for `filter.RegisterHttpFilter`
+or `filter.RegisterNetworkFilterPlugin`, and compares the resulting set
+against the blank-import list. It prints the missing lines. It does NOT
+edit the registry file — apply the diff by hand (this is a high-blast-
+radius file; a human should read the insertion point).
+
+Run it as the last step of every filter PR.
Review Comment:
加一个脚本来检测有点多余了吧，就是检查一下的事，agent完全有能力自己检测的
##########
skills/pixiu-filter-author/SKILL.md:
##########
@@ -0,0 +1,366 @@
+---
+name: pixiu-filter-author
+description: |
+ Create a new HTTP or Network filter for Apache dubbo-go-pixiu. Use whenever
+ the user wants to add a custom filter, extend gateway per-request behavior,
+ implement a new auth/logging/transformation/proxy filter, or mentions
+ "pixiu filter", "dgp.filter", "http_filters", "filter chain", "custom
filter",
+ "extend pixiu", or is porting a filter from Envoy/Kong/APISIX to pixiu.
+ Strongly prefer this skill over ad-hoc generation even when the user does
+ not say the word "skill" — getting the SPI registration right is the single
+ biggest source of "my filter does not run" bug reports.
+allowed-tools: [Read, Grep, Glob, Edit, Write, Bash]
+metadata:
+ version: "0.1.2"
+ domain: extension
+ scope: implementation
+ triggers: ["filter", "pixiu filter", "dgp.filter", "http_filters", "filter
chain", "custom filter", "extend pixiu"]
+ pixiu_min_version: "0.6.0"
+ role: specialist
+---
+
+# pixiu-filter-author — Authoring HTTP / Network Filters
+
+dubbo-go-pixiu's extension surface is Envoy-style: every filter is a Go
+package that registers a `Plugin` in `init()`, and a central file
+(`pkg/pluginregistry/registry.go`) blank-imports all such packages to
+activate them. A single missing blank import is the most common reason a
+newly written filter "does nothing" — so this skill treats registration as
+a first-class concern, not an afterthought.
+
+## When to Use
+
+Use this skill when the user wants to:
+- Add a new HTTP filter (auth, logging, header rewriting, rate limiting,
+ request transformation, metric emission, etc.).
+- Add a new Network (L4) filter (e.g. a custom connection manager).
+- Port a filter from Envoy / Kong / APISIX onto pixiu.
+- Fix an existing filter that "isn't running" — start at Step 5 (blank
+ import) and Step 3 (phase).
+
+**Do NOT use this skill for:**
+- Editing the yaml that *uses* an existing filter → that is plain config
+ work. Only include a minimal mounting snippet when you create a new
+ filter in this skill.
+- Mounting an already-registered filter such as CORS, JWT, OPA, MCP,
+ LLM proxy, or Dubbo proxy. In that case do not scaffold code and do
+ not touch `pkg/pluginregistry/registry.go`; answer with the smallest
+ `http_filters` snippet and state that the filter already exists.
+- Adding a registry adapter (Nacos, ZK, Consul), a new protocol
+ listener (WebSocket, MQTT), or a custom load-balancing algorithm —
+ these are different SPI hubs and out of scope for this skill. Point
+ the user to the relevant interface file
+ (`pkg/common/extension/adapter/adapter.go`,
+ `pkg/listener/listener.go`, or
+ `pkg/cluster/loadbalancer/`) and let them work directly.
+
+## Prerequisites
+
+- pixiu ≥ 0.6.0. Verify with `grep dubbo-go-pixiu go.mod` in Step 0.
+- The user should be inside a clone of `github.com/apache/dubbo-go-pixiu`
+ (or a fork). If they are not, stop and ask — this skill edits repo files.
+
+## Steps
+
+### Step 0 — Verify Context (ALWAYS FIRST, no exceptions)
+
+Before writing a single line of code, read the current shape of three
+files. pixiu's SPI is stable but not frozen, and your instinct about
+these interfaces can be one minor version stale:
+
+1. `pkg/common/extension/filter/filter.go` — the four interfaces
+ (`HttpFilterPlugin`, `HttpFilterFactory`, `HttpDecodeFilter`,
+ `HttpEncodeFilter`), plus `NetworkFilterPlugin` / `NetworkFilter` for
+ L4. Read signatures, especially `PrepareFilterChain`.
+2. `pkg/pluginregistry/registry.go` — the blank-import list you will
+ extend in Step 5. Note the alphabetical order within groups.
+3. `pkg/filter/cors/cors.go` — the shortest idiomatic HTTP filter in the
+ tree. A single file, all four types (`Plugin`, `FilterFactory`,
+ `Filter`, `Config`) declared together. Note: `cors` ships **without**
+ a `_test.go` — that matches roughly half of the existing filter
+ packages. See Step 7 for the project's actual convention.
+
+Then `ls pkg/filter/` and `ls pkg/filter/http/` to see the existing
+directory conventions. Do not assume — layouts evolve (e.g. `cors` lives
+at `pkg/filter/cors/`, while proxy filters live at
+`pkg/filter/http/httpproxy/`, `pkg/filter/http/dubboproxy/`).
+
+Load `references/filter-interface.md` if you need a side-by-side of the
+four interfaces with their pixiu source-line anchors.
+
+### Step 1 — Clarify the Filter Shape (STOP and ask)
+
+Do not write code until the user has answered, in plain words:
+
+1. **Phase**: Decode (request, before upstream), Encode (response, after
+ upstream), or both?
+2. **Kind**: the string that identifies the filter in yaml. It MUST start
+ with `dgp.filter.http.` for HTTP filters or
+ match Pixiu's current Network filter constants for L4 filters, usually
+ `dgp.filter.network.`. The built-in HTTP connection manager is the
+ special network-filter Kind `dgp.filter.httpconnectionmanager`.
+ Suggest a name after checking `pkg/common/constant/key.go`; confirm
+ the user is happy.
+3. **Config fields**: what yaml keys does the user want? (`allow_origin`,
+ `max_request_bytes`, etc.)
+4. **Where to put the package**: the convention is
+ `pkg/filter/<name>/` for ordinary filters and
+ `pkg/filter/http/<name>/` for *proxy-style* filters (the ones that
+ actually call upstream). If unsure, ask — getting this right once
+ beats moving files later.
+
+If the user says "just write something reasonable", pick a minimal
+request-logging filter as the placeholder and be explicit about the
+assumptions in a short summary before coding.
+
+### Step 2 — Scaffold the Package
+
+Create the directory and initial files. Two shapes are idiomatic in the
+existing tree:
+
+- **Single file** (cors, csrf, jwt, header, host, tracing): everything
+ lives in `<name>.go`. This is the dominant pattern for non-proxy
+ filters — about 20 of the existing filter packages follow it.
+- **Split files** (network/dubboproxy, mcp/mcpserver, ai/kvcache, etc.):
+ `plugin.go`, `filter.go`, `config.go`. Use this only when the package
+ has more than ~200 LOC or several distinct types worth separating.
+
+Test files (`*_test.go`) are **not** part of either default shape —
+they are added on a per-filter basis when the logic warrants it. See
+Step 7.
+
+Optionally run `bash scripts/scaffold-filter.sh <name>` from this skill
+— it emits a single-file skeleton (no test file by default, matching
+the common case). Do NOT treat the scaffolder output as final; the
+user still owns every Config field.
+
+### Step 3 — Implement the Four Interfaces
+
+The four types for an HTTP filter, in order:
+
+```go
+type Plugin struct{}
+func (p *Plugin) Kind() string { return Kind }
+func (p *Plugin) CreateFilterFactory() (filter.HttpFilterFactory, error) {
+ return &FilterFactory{cfg: &Config{}}, nil
+}
+
+type FilterFactory struct{ cfg *Config }
+func (f *FilterFactory) Config() any { return f.cfg }
+func (f *FilterFactory) Apply() error { /* validate / defaults */ return nil }
+func (f *FilterFactory) PrepareFilterChain(ctx *http.HttpContext, chain
filter.FilterChain) error {
+ inst := &Filter{cfg: f.cfg.DeepCopy()} // or a shallow copy of the pieces
you need
+ chain.AppendDecodeFilters(inst) // or AppendEncodeFilters, or both
+ return nil
+}
+
+type Filter struct{ cfg *Config }
+func (f *Filter) Decode(ctx *http.HttpContext) filter.FilterStatus { return
filter.Continue }
+// and/or:
+func (f *Filter) Encode(ctx *http.HttpContext) filter.FilterStatus { return
filter.Continue }
+```
+
+A few subtle rules the interface alone does not enforce:
+
+- **Do not reuse `factory.cfg` directly in the Filter instance.** The
+ factory's config pointer can be hot-reloaded at runtime. Copy the
+ fields you need into the Filter when `PrepareFilterChain` runs. The
+ CORS filter's `factory.cfg.DeepCopy()` pattern is the canonical
+ reference.
+- **Decode vs Encode is about the phase, not the direction.** Mutating
+ `ctx.TargetResp` (the outbound body) from a Decode filter is a
+ category error — read `references/context-api.md` before touching
+ response state.
+- **Response-body mutation is an Encode-only checklist.** For redaction,
+ watermarking, compression, or similar response transforms, load
+ `references/context-api.md` and `references/testing-patterns.md`.
+ Operate on `ctx.TargetResp` only after confirming the concrete
+ response type. Handle `*client.UnaryResponse` deliberately, pass
+ streaming responses through unless the user explicitly asked for
+ streaming support, gate by `Content-Type` when the behavior is textual,
+ and write table-driven tests for empty body, unsupported content type,
+ configured/default values, and invalid config.
+
+For Network filters, the interface surface is larger
+(`ServeHTTP`, `OnData`, `OnTripleData`, etc.); reuse an existing one
+(`pkg/filter/network/httpconnectionmanager/`) as a template.
+
+### Step 4 — Register in `init()`
+
+```go
+func init() {
+ filter.RegisterHttpFilter(&Plugin{})
+}
+```
+
+For Network filters the call is
`filter.RegisterNetworkFilterPlugin(&Plugin{})`.
+
+### Step 5 — Add the Blank Import (THIS is where filters die silently)
+
+Edit `pkg/pluginregistry/registry.go` and insert, in alphabetical order
+within the correct grouping:
+
+```go
+_ "github.com/apache/dubbo-go-pixiu/pkg/filter/<name>"
+```
+
+(Or `.../pkg/filter/http/<name>` if the package lives there.)
+
+If this line is missing, the filter compiles fine, `go test` passes, and
+pixiu boots — but the filter never registers, and yaml using its Kind
+fails with `no filter found for name ...`. This is *the* pixiu rite of
+passage.
+
+You can run `bash scripts/gen-registry-import.sh` to auto-scan
+`pkg/filter/` and `pkg/filter/http/` for packages that contain
+`filter.RegisterHttpFilter` but are not yet blank-imported. The script
+prints a diff; review it, then apply.
Review Comment:
ditto
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
