From 01c960104491fbe280a0a9baef8d2475b9d2e556 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Mon, 1 Jun 2026 19:39:31 -0400 Subject: [PATCH 1/3] =?UTF-8?q?refactor(authbridge):=20simplify=20PluginCa?= =?UTF-8?q?pabilities=20=E2=80=94=20remove=20unused=20fields?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove five fields that were either deprecated or speculative infrastructure with no current plugin usage: - BodyAccess: deprecated alias for ReadsBody - Reads: no plugin declared it; validation was never triggered - Writes: useless without Reads — the slot contract only works as a pair - After: soft ordering; only appeared in tests, no plugin used it - Claims: mutual exclusion; only appeared in tests, no plugin used it Removing Reads/Writes also eliminates the defaultSlots map, WithSlots option, and the slot-name/prior-write validation block in validateCapabilities(). That function now only enforces body-mutation ordering rules. Result: PluginCapabilities goes from 9 fields to 5 (ReadsBody, WritesBody, Requires, RequiresAny, Description). The interface a plugin author must understand is materially smaller. Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/authlib/contracts/claims.go | 29 ---- .../authlib/listener/extproc/server_test.go | 2 +- .../listener/forwardproxy/server_test.go | 2 +- .../listener/reverseproxy/server_test.go | 2 +- .../authlib/pipeline/bodymutation_test.go | 10 +- .../authlib/pipeline/configured_test.go | 10 +- authbridge/authlib/pipeline/holder_test.go | 2 +- authbridge/authlib/pipeline/pipeline.go | 72 ++------- authbridge/authlib/pipeline/pipeline_test.go | 115 +------------- authbridge/authlib/pipeline/plugin.go | 75 ++------- .../authlib/plugins/a2aparser/plugin.go | 1 - .../authlib/plugins/a2aparser/plugin_test.go | 3 - .../authlib/plugins/inferenceparser/plugin.go | 1 - .../plugins/inferenceparser/plugin_test.go | 3 - .../authlib/plugins/jwtvalidation/plugin.go | 1 - .../authlib/plugins/mcpparser/plugin.go | 1 - .../authlib/plugins/mcpparser/plugin_test.go | 3 - .../plugins/plugintesting/plugintesting.go | 2 +- authbridge/authlib/plugins/registry.go | 56 ++----- authbridge/authlib/plugins/registry_test.go | 149 ++---------------- .../authlib/sessionapi/catalog_adapter.go | 4 - authbridge/authlib/sessionapi/server.go | 35 ++-- authbridge/authlib/sessionapi/server_test.go | 27 ++-- 23 files changed, 80 insertions(+), 525 deletions(-) delete mode 100644 authbridge/authlib/contracts/claims.go diff --git a/authbridge/authlib/contracts/claims.go b/authbridge/authlib/contracts/claims.go deleted file mode 100644 index 59458fa74..000000000 --- a/authbridge/authlib/contracts/claims.go +++ /dev/null @@ -1,29 +0,0 @@ -package contracts - -// Claim strings for PluginCapabilities.Claims. A claim is a semantic -// resource that exactly one plugin per chain owns; two plugins -// declaring the same claim cause plugins.Build to fail at startup. -// -// Plugin authors reference these constants instead of string literals -// so typos are compile errors and the canonical set is greppable. -// Third-party plugins that need a claim not listed here may declare -// their own arbitrary string — the framework enforces uniqueness of -// whatever it sees, not "must be from the list" — but won't benefit -// from typo safety until the claim is upstreamed here. -// -// Resist speculative additions. Each constant is a small -// hard-to-deprecate contract. Adding a new claim should be driven by -// a concrete use case where two plugins would conflict in practice. -// Upstream a new constant in a follow-up PR when a claim stabilizes, -// with a godoc paragraph explaining what the resource is and which -// in-tree plugins claim it. - -// ClaimAuthorizationHeader is the exclusive right to replace the -// outbound Authorization header. token-exchange and token-broker -// both claim this; they cannot coexist in the same outbound chain -// because the one that runs second would silently clobber the -// first's work. -// -// A future SPIFFE-exchanger or Keycloak-flavored gate plugin that -// also rewrites Authorization should declare this claim. -const ClaimAuthorizationHeader = "authorization_header" diff --git a/authbridge/authlib/listener/extproc/server_test.go b/authbridge/authlib/listener/extproc/server_test.go index b968b3998..4d335822a 100644 --- a/authbridge/authlib/listener/extproc/server_test.go +++ b/authbridge/authlib/listener/extproc/server_test.go @@ -388,7 +388,7 @@ type bodyRecorderPlugin struct { func (p *bodyRecorderPlugin) Name() string { return "body-recorder" } func (p *bodyRecorderPlugin) Capabilities() pipeline.PluginCapabilities { - return pipeline.PluginCapabilities{BodyAccess: true} + return pipeline.PluginCapabilities{ReadsBody: true} } func (p *bodyRecorderPlugin) OnRequest(_ context.Context, pctx *pipeline.Context) pipeline.Action { p.receivedBody = pctx.Body diff --git a/authbridge/authlib/listener/forwardproxy/server_test.go b/authbridge/authlib/listener/forwardproxy/server_test.go index 3c2f8bf0a..0eb801ced 100644 --- a/authbridge/authlib/listener/forwardproxy/server_test.go +++ b/authbridge/authlib/listener/forwardproxy/server_test.go @@ -313,7 +313,7 @@ type bodyRecorderPlugin struct { func (p *bodyRecorderPlugin) Name() string { return "body-recorder" } func (p *bodyRecorderPlugin) Capabilities() pipeline.PluginCapabilities { - return pipeline.PluginCapabilities{BodyAccess: true} + return pipeline.PluginCapabilities{ReadsBody: true} } func (p *bodyRecorderPlugin) OnRequest(_ context.Context, pctx *pipeline.Context) pipeline.Action { p.receivedBody = pctx.Body diff --git a/authbridge/authlib/listener/reverseproxy/server_test.go b/authbridge/authlib/listener/reverseproxy/server_test.go index a0e00def0..b418eee5f 100644 --- a/authbridge/authlib/listener/reverseproxy/server_test.go +++ b/authbridge/authlib/listener/reverseproxy/server_test.go @@ -118,7 +118,7 @@ type bodyRecorderPlugin struct { func (p *bodyRecorderPlugin) Name() string { return "body-recorder" } func (p *bodyRecorderPlugin) Capabilities() pipeline.PluginCapabilities { - return pipeline.PluginCapabilities{BodyAccess: true} + return pipeline.PluginCapabilities{ReadsBody: true} } func (p *bodyRecorderPlugin) OnRequest(_ context.Context, pctx *pipeline.Context) pipeline.Action { p.receivedBody = pctx.Body diff --git a/authbridge/authlib/pipeline/bodymutation_test.go b/authbridge/authlib/pipeline/bodymutation_test.go index 9949bd9f8..0ea97a00f 100644 --- a/authbridge/authlib/pipeline/bodymutation_test.go +++ b/authbridge/authlib/pipeline/bodymutation_test.go @@ -7,9 +7,8 @@ import ( "testing" ) -// TestCapabilities_Normalize covers the compatibility rules: deprecated -// BodyAccess folds into ReadsBody, and WritesBody auto-promotes to -// ReadsBody so a mutator always satisfies the "must have read" invariant. +// TestCapabilities_Normalize: WritesBody auto-promotes to ReadsBody so a +// mutator always satisfies the "must have read" invariant. func TestCapabilities_Normalize(t *testing.T) { tests := []struct { name string @@ -17,11 +16,6 @@ func TestCapabilities_Normalize(t *testing.T) { wantReads bool wantWrites bool }{ - { - name: "BodyAccess alias folds into ReadsBody", - in: PluginCapabilities{BodyAccess: true}, - wantReads: true, - }, { name: "WritesBody implies ReadsBody", in: PluginCapabilities{WritesBody: true}, diff --git a/authbridge/authlib/pipeline/configured_test.go b/authbridge/authlib/pipeline/configured_test.go index 83fcf4f96..7aadf44cf 100644 --- a/authbridge/authlib/pipeline/configured_test.go +++ b/authbridge/authlib/pipeline/configured_test.go @@ -42,7 +42,7 @@ func TestConfiguredPluginRawConfig(t *testing.T) { func TestConfiguredPluginPassesThroughPluginMethods(t *testing.T) { fake := &fakePlugin{ name: "jwt-validation", - caps: PluginCapabilities{Reads: []string{"a"}, Writes: []string{"security"}}, + caps: PluginCapabilities{ReadsBody: true, Description: "test plugin"}, } cp := WrapConfigured(fake, json.RawMessage(`{}`)) @@ -50,11 +50,11 @@ func TestConfiguredPluginPassesThroughPluginMethods(t *testing.T) { t.Fatalf("Name pass-through broken: %q", cp.Name()) } caps := cp.Capabilities() - if len(caps.Reads) != 1 || caps.Reads[0] != "a" { - t.Fatalf("Capabilities pass-through broken: %+v", caps) + if !caps.ReadsBody { + t.Fatalf("Capabilities pass-through broken: ReadsBody should be true, got %+v", caps) } - if len(caps.Writes) != 1 || caps.Writes[0] != "security" { - t.Fatalf("Capabilities pass-through broken: %+v", caps) + if caps.Description != "test plugin" { + t.Fatalf("Capabilities pass-through broken: Description mismatch, got %+v", caps) } cp.OnRequest(context.Background(), nil) cp.OnResponse(context.Background(), nil) diff --git a/authbridge/authlib/pipeline/holder_test.go b/authbridge/authlib/pipeline/holder_test.go index 2c0e23219..2afdb613c 100644 --- a/authbridge/authlib/pipeline/holder_test.go +++ b/authbridge/authlib/pipeline/holder_test.go @@ -21,7 +21,7 @@ type holderTestPlugin struct { func (p *holderTestPlugin) Name() string { return p.name } func (p *holderTestPlugin) Capabilities() PluginCapabilities { - return PluginCapabilities{BodyAccess: p.needsBody} + return PluginCapabilities{ReadsBody: p.needsBody} } func (p *holderTestPlugin) OnRequest(_ context.Context, _ *Context) Action { p.runCount.Add(1) diff --git a/authbridge/authlib/pipeline/pipeline.go b/authbridge/authlib/pipeline/pipeline.go index e3cc0cd0d..a741882c8 100644 --- a/authbridge/authlib/pipeline/pipeline.go +++ b/authbridge/authlib/pipeline/pipeline.go @@ -28,33 +28,14 @@ type Pipeline struct { finishTimeout time.Duration } -// defaultSlots lists the built-in extension slot names. -var defaultSlots = map[string]bool{ - "mcp": true, - "a2a": true, - "security": true, - "delegation": true, - "inference": true, - "custom": true, -} - // Option configures pipeline construction. type Option func(*options) type options struct { - extraSlots []string policies []ErrorPolicy finishTimeout time.Duration } -// WithSlots registers additional valid extension slot names beyond the built-in set. -// Use this when a bridge plugin (e.g., CPEX) produces extensions not in the default set. -func WithSlots(slots ...string) Option { - return func(o *options) { - o.extraSlots = append(o.extraSlots, slots...) - } -} - // WithFinishTimeout overrides the per-plugin OnFinish timeout. Each // plugin's OnFinish runs under a fresh ctx derived from // context.Background() with this timeout applied; a zero or negative @@ -79,21 +60,13 @@ func WithPolicies(policies ...ErrorPolicy) Option { } } -// New creates a Pipeline from the given plugins after validating capability wiring. -// Returns an error if any plugin declares a read on a slot that no earlier plugin writes. +// New creates a Pipeline from the given plugins after validating body-access rules. func New(plugins []Plugin, opts ...Option) (*Pipeline, error) { var o options for _, opt := range opts { opt(&o) } - validSlots := make(map[string]bool, len(defaultSlots)+len(o.extraSlots)) - for k, v := range defaultSlots { - validSlots[k] = v - } - for _, s := range o.extraSlots { - validSlots[s] = true - } - if err := validateCapabilities(plugins, validSlots); err != nil { + if err := validateCapabilities(plugins); err != nil { return nil, err } if len(o.policies) > len(plugins) { @@ -318,8 +291,6 @@ func (p *Pipeline) NotReadyPlugin() string { // NeedsBody returns true if any plugin in the pipeline needs the body // buffered — either to read it (ReadsBody) or to mutate it (WritesBody). -// Normalize() folds the deprecated BodyAccess alias into ReadsBody, so -// both legacy and modern plugins are covered by the single check. func (p *Pipeline) NeedsBody() bool { for _, plugin := range p.plugins { caps := plugin.Capabilities().Normalize() @@ -501,44 +472,23 @@ func (p *Pipeline) dispatchFinish(parent context.Context, name string, f Finishe f.OnFinish(ctx, pctx) } -// validateCapabilities checks that every slot a plugin reads has been written -// by an earlier plugin in the chain, and applies the body-mutation rules: -// - At most one WritesBody plugin per pipeline (direction-scoped). -// Mutation ordering would otherwise be ambiguous; downstream readers -// can't tell which version they're seeing. -// - A body mutator must not run before a body reader. Readers that -// declared ReadsBody expect to see the original bytes; placing a -// mutator earlier would silently change what they observe. -func validateCapabilities(plugins []Plugin, validSlots map[string]bool) error { - written := make(map[string]bool) - var mutatorName string // set once the first WritesBody plugin is seen - var readerAfterMutator string // non-empty if a ReadsBody plugin follows the mutator +// validateCapabilities enforces body-mutation ordering rules: +// - At most one WritesBody plugin per pipeline — mutation ordering would +// otherwise be ambiguous; downstream readers can't tell which version +// they're seeing. +// - A body reader (ReadsBody) must not follow a body mutator (WritesBody) — +// the reader would silently see mutated bytes instead of the originals. +func validateCapabilities(plugins []Plugin) error { + var mutatorName string + var readerAfterMutator string for _, plugin := range plugins { caps := plugin.Capabilities().Normalize() - for _, slot := range caps.Reads { - if !validSlots[slot] { - return fmt.Errorf("plugin %q declares read on unknown slot %q", plugin.Name(), slot) - } - if !written[slot] { - return fmt.Errorf("plugin %q reads slot %q but no earlier plugin writes it", plugin.Name(), slot) - } - } - for _, slot := range caps.Writes { - if !validSlots[slot] { - return fmt.Errorf("plugin %q declares write on unknown slot %q", plugin.Name(), slot) - } - written[slot] = true - } if caps.WritesBody { if mutatorName != "" { return fmt.Errorf("pipeline: two plugins declare WritesBody: %q and %q — mutation ordering would be ambiguous; at most one body mutator per pipeline is allowed", mutatorName, plugin.Name()) } mutatorName = plugin.Name() } else if caps.ReadsBody && mutatorName != "" && readerAfterMutator == "" { - // ReadsBody-only plugin running AFTER a WritesBody plugin - // would see the mutated bytes, which surprises the reader. - // Stash the first occurrence; validated below so the error - // names both plugins involved. readerAfterMutator = plugin.Name() } } diff --git a/authbridge/authlib/pipeline/pipeline_test.go b/authbridge/authlib/pipeline/pipeline_test.go index ab3f72406..54548e212 100644 --- a/authbridge/authlib/pipeline/pipeline_test.go +++ b/authbridge/authlib/pipeline/pipeline_test.go @@ -54,7 +54,6 @@ func TestPipelineRun_Sequential(t *testing.T) { } p2 := &stubPlugin{ name: "second", - caps: PluginCapabilities{Reads: []string{"custom"}}, onReq: func(_ context.Context, pctx *Context) Action { order = append(order, "second") if pctx.Extensions.Custom["key"] != "value" { @@ -63,7 +62,6 @@ func TestPipelineRun_Sequential(t *testing.T) { return Action{Type: Continue} }, } - p1.caps = PluginCapabilities{Writes: []string{"custom"}} pipe, err := New([]Plugin{p1, p2}) if err != nil { @@ -183,74 +181,6 @@ func TestPipelineRunResponse_Reject(t *testing.T) { } } -func TestNew_ValidCapabilities(t *testing.T) { - plugins := []Plugin{ - &stubPlugin{ - name: "writer", - caps: PluginCapabilities{Writes: []string{"mcp"}}, - }, - &stubPlugin{ - name: "reader", - caps: PluginCapabilities{Reads: []string{"mcp"}}, - }, - } - _, err := New(plugins) - if err != nil { - t.Errorf("expected no error, got: %v", err) - } -} - -func TestNew_InvalidCapabilities_ReadBeforeWrite(t *testing.T) { - plugins := []Plugin{ - &stubPlugin{ - name: "reader", - caps: PluginCapabilities{Reads: []string{"mcp"}}, - }, - &stubPlugin{ - name: "writer", - caps: PluginCapabilities{Writes: []string{"mcp"}}, - }, - } - _, err := New(plugins) - if err == nil { - t.Fatal("expected error for read-before-write, got nil") - } -} - -func TestNew_InvalidCapabilities_UnknownSlot(t *testing.T) { - plugins := []Plugin{ - &stubPlugin{ - name: "bad-reader", - caps: PluginCapabilities{Reads: []string{"nonexistent"}}, - }, - } - _, err := New(plugins) - if err == nil { - t.Fatal("expected error for unknown slot, got nil") - } -} - -func TestNew_MultipleWriters(t *testing.T) { - plugins := []Plugin{ - &stubPlugin{ - name: "writer-1", - caps: PluginCapabilities{Writes: []string{"security"}}, - }, - &stubPlugin{ - name: "writer-2", - caps: PluginCapabilities{Writes: []string{"security"}}, - }, - &stubPlugin{ - name: "reader", - caps: PluginCapabilities{Reads: []string{"security"}}, - }, - } - _, err := New(plugins) - if err != nil { - t.Errorf("multiple writers should be valid, got: %v", err) - } -} - func TestNew_NoCapabilities(t *testing.T) { plugins := []Plugin{ &stubPlugin{name: "simple"}, @@ -261,47 +191,6 @@ func TestNew_NoCapabilities(t *testing.T) { } } -func TestNew_CustomSlot(t *testing.T) { - plugins := []Plugin{ - &stubPlugin{ - name: "custom-writer", - caps: PluginCapabilities{Writes: []string{"custom"}}, - }, - &stubPlugin{ - name: "custom-reader", - caps: PluginCapabilities{Reads: []string{"custom"}}, - }, - } - _, err := New(plugins) - if err != nil { - t.Errorf("custom slot should be valid, got: %v", err) - } -} - -func TestNew_WithSlots(t *testing.T) { - plugins := []Plugin{ - &stubPlugin{ - name: "cpex-bridge", - caps: PluginCapabilities{Writes: []string{"cpex.completion"}}, - }, - &stubPlugin{ - name: "consumer", - caps: PluginCapabilities{Reads: []string{"cpex.completion"}}, - }, - } - // Without WithSlots, this should fail - _, err := New(plugins) - if err == nil { - t.Fatal("expected error for unregistered slot without WithSlots") - } - - // With WithSlots, this should succeed - _, err = New(plugins, WithSlots("cpex.completion")) - if err != nil { - t.Errorf("expected no error with WithSlots, got: %v", err) - } -} - func TestDelegationExtension_AppendHop(t *testing.T) { d := &DelegationExtension{} @@ -379,8 +268,8 @@ func TestPipelineRun_ContextCancellation(t *testing.T) { } func TestPipeline_Plugins_ReturnsCopy(t *testing.T) { - a := &stubPlugin{name: "a", caps: PluginCapabilities{Writes: []string{"custom"}}} - b := &stubPlugin{name: "b", caps: PluginCapabilities{Reads: []string{"custom"}}} + a := &stubPlugin{name: "a"} + b := &stubPlugin{name: "b"} p, err := New([]Plugin{a, b}) if err != nil { t.Fatalf("New: %v", err) diff --git a/authbridge/authlib/pipeline/plugin.go b/authbridge/authlib/pipeline/plugin.go index e149feb4c..094450603 100644 --- a/authbridge/authlib/pipeline/plugin.go +++ b/authbridge/authlib/pipeline/plugin.go @@ -10,18 +10,14 @@ type Plugin interface { OnResponse(ctx context.Context, pctx *Context) Action } -// PluginCapabilities declares what extension slots a plugin reads and -// writes, plus whether it accesses the request / response body. +// PluginCapabilities declares whether a plugin accesses the request / +// response body and which other plugins it depends on. // -// The pipeline validates at startup that all Reads are satisfied by an -// earlier plugin's Writes. Body-access fields drive the listener's -// body-buffering handshake (ext_proc ProcessingMode, net/http read-body). +// Body-access fields drive the listener's body-buffering handshake +// (ext_proc ProcessingMode, net/http read-body). Dependency fields are +// checked at startup by plugins.Build so misconfigured chains fail before +// traffic arrives. type PluginCapabilities struct { - // Reads / Writes name extension slots (A2A, MCP, Inference, Custom - // map keys). Checked at pipeline.New. - Reads []string - Writes []string - // ReadsBody: the plugin reads pctx.Body in OnRequest and/or // pctx.ResponseBody in OnResponse. The listener buffers the body // when any plugin declares this; without it, pctx.Body is nil and @@ -41,14 +37,6 @@ type PluginCapabilities struct { // process boot. WritesBody bool - // BodyAccess is a deprecated alias for ReadsBody, kept so existing - // plugins compile unchanged through one release. Normalize() folds - // BodyAccess into ReadsBody before validation and listener - // negotiation read the normalized fields. - // - // Deprecated: use ReadsBody. Will be removed in a future release. - BodyAccess bool - // Requires names plugins that MUST be present in the same chain // AND appear earlier (lower index). Matches are case-sensitive // plugin Name() strings. A missing or misordered name causes @@ -57,9 +45,7 @@ type PluginCapabilities struct { // Use Requires when the plugin hardcodes access to a specific // other plugin's extension fields — e.g., a tool-allowlist plugin // that reads pctx.Extensions.MCP.Params["name"] declares - // Requires: []string{"mcp-parser"}. If the plugin instead reads - // through pctx.ContentSources() and works against any parser, - // see RequiresAny. + // Requires: []string{"mcp-parser"}. Requires []string // RequiresAny names plugins of which AT LEAST ONE must be present @@ -68,59 +54,28 @@ type PluginCapabilities struct { // causes plugins.Build to fail at startup. // // Use RequiresAny for protocol-agnostic plugins that read through - // pctx.ContentSources(). Example: a PII scrubber that consumes - // fragments from whatever parsers are wired in declares - // RequiresAny: []string{"a2a-parser", "mcp-parser", "inference-parser"}. - // That way a chain with no parsers fails loud instead of running - // the guardrail as silent dead code. + // pctx.ContentSources(). Example: a guardrail that works against any + // parser declares RequiresAny: []string{"a2a-parser", "mcp-parser", + // "inference-parser"} so a chain with no parsers fails loud instead + // of running the guardrail as silent dead code. RequiresAny []string - // After names plugins that, IF present in the same chain, must - // appear earlier. Unlike Requires/RequiresAny, a missing name is - // not an error — After is a soft ordering hint. Useful for - // plugins that benefit from earlier state being populated but - // degrade gracefully without it. - After []string - - // Claims declares semantic resources the plugin takes exclusive - // ownership of. Within a single chain, at most one plugin may - // declare any given claim string; two plugins with an overlapping - // claim cause plugins.Build to fail at startup. - // - // Claim strings are arbitrary but authors should prefer the - // constants in authlib/contracts/ (e.g. contracts.ClaimAuthorizationHeader) - // so typos are compile errors and the canonical set is greppable. - // Third-party plugins may declare their own strings; the framework - // enforces uniqueness of whatever it sees, not "must be from the - // list." See authlib/contracts/claims.go for the canonical - // vocabulary. - Claims []string - // Description is operator-facing prose, one line, ≤80 chars, // describing what this plugin does. Surfaces in `abctl`'s - // plugin-detail and catalog panes, and in /v1/plugins. Empty for - // plugins that haven't opted in; new plugins should populate it. + // plugin-detail and catalog panes, and in /v1/plugins. // // Capabilities are static type-level metadata: Capabilities() must // return the same value for any instance produced by a given - // factory. The catalog endpoint relies on this — varying capabilities - // by instance state silently produces wrong catalog entries. If a - // plugin's behavior varies enough that its capabilities differ, - // register it under multiple names. + // factory. If a plugin's behavior varies enough that its capabilities + // differ, register it under multiple names. Description string } -// Normalize applies compatibility rules to a PluginCapabilities: -// - BodyAccess (deprecated) is folded into ReadsBody. -// - WritesBody implies ReadsBody (you can't mutate what you didn't see). -// +// Normalize applies WritesBody-implies-ReadsBody promotion. // Called by Pipeline.New for every plugin's declared capabilities so the // rest of the framework reads a normalized form. Plugins never need to // call this themselves. func (c PluginCapabilities) Normalize() PluginCapabilities { - if c.BodyAccess { - c.ReadsBody = true - } if c.WritesBody { c.ReadsBody = true } diff --git a/authbridge/authlib/plugins/a2aparser/plugin.go b/authbridge/authlib/plugins/a2aparser/plugin.go index 28cf9f5b5..c40ab130c 100644 --- a/authbridge/authlib/plugins/a2aparser/plugin.go +++ b/authbridge/authlib/plugins/a2aparser/plugin.go @@ -26,7 +26,6 @@ func (p *A2AParser) Name() string { return "a2a-parser" } func (p *A2AParser) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ - Writes: []string{"a2a"}, ReadsBody: true, Description: "Parses A2A messages into pctx.Extensions.A2A for downstream plugins.", } diff --git a/authbridge/authlib/plugins/a2aparser/plugin_test.go b/authbridge/authlib/plugins/a2aparser/plugin_test.go index 47e6b12d4..4abf0a6ff 100644 --- a/authbridge/authlib/plugins/a2aparser/plugin_test.go +++ b/authbridge/authlib/plugins/a2aparser/plugin_test.go @@ -18,9 +18,6 @@ func TestA2AParser_Capabilities(t *testing.T) { if !caps.ReadsBody { t.Error("ReadsBody should be true") } - if len(caps.Writes) != 1 || caps.Writes[0] != "a2a" { - t.Errorf("Writes = %v, want [a2a]", caps.Writes) - } } func TestA2AParser_MessageSend(t *testing.T) { diff --git a/authbridge/authlib/plugins/inferenceparser/plugin.go b/authbridge/authlib/plugins/inferenceparser/plugin.go index cbdea0b78..876c82d0c 100644 --- a/authbridge/authlib/plugins/inferenceparser/plugin.go +++ b/authbridge/authlib/plugins/inferenceparser/plugin.go @@ -26,7 +26,6 @@ func (p *InferenceParser) Name() string { return "inference-parser" } func (p *InferenceParser) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ - Writes: []string{"inference"}, ReadsBody: true, Description: "Parses LLM completions into pctx.Extensions.Inference.", } diff --git a/authbridge/authlib/plugins/inferenceparser/plugin_test.go b/authbridge/authlib/plugins/inferenceparser/plugin_test.go index 4723644df..b9e961b6c 100644 --- a/authbridge/authlib/plugins/inferenceparser/plugin_test.go +++ b/authbridge/authlib/plugins/inferenceparser/plugin_test.go @@ -18,9 +18,6 @@ func TestInferenceParser_Capabilities(t *testing.T) { if !caps.ReadsBody { t.Error("ReadsBody should be true") } - if len(caps.Writes) != 1 || caps.Writes[0] != "inference" { - t.Errorf("Writes = %v, want [inference]", caps.Writes) - } } func TestInferenceParser_ChatCompletions(t *testing.T) { diff --git a/authbridge/authlib/plugins/jwtvalidation/plugin.go b/authbridge/authlib/plugins/jwtvalidation/plugin.go index 72c147879..f422c2f22 100644 --- a/authbridge/authlib/plugins/jwtvalidation/plugin.go +++ b/authbridge/authlib/plugins/jwtvalidation/plugin.go @@ -215,7 +215,6 @@ func (p *JWTValidation) Name() string { return "jwt-validation" } func (p *JWTValidation) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ - Writes: []string{"security"}, Description: "Inbound JWT validation (signature, issuer, audience) against JWKS.", } } diff --git a/authbridge/authlib/plugins/mcpparser/plugin.go b/authbridge/authlib/plugins/mcpparser/plugin.go index a423890df..e5c3069f1 100644 --- a/authbridge/authlib/plugins/mcpparser/plugin.go +++ b/authbridge/authlib/plugins/mcpparser/plugin.go @@ -87,7 +87,6 @@ func (p *MCPParser) Name() string { return "mcp-parser" } func (p *MCPParser) Capabilities() pipeline.PluginCapabilities { return pipeline.PluginCapabilities{ - Writes: []string{"mcp"}, ReadsBody: true, Description: "Parses MCP tool calls/results into pctx.Extensions.MCP.", } diff --git a/authbridge/authlib/plugins/mcpparser/plugin_test.go b/authbridge/authlib/plugins/mcpparser/plugin_test.go index 433c5a78e..16eca3889 100644 --- a/authbridge/authlib/plugins/mcpparser/plugin_test.go +++ b/authbridge/authlib/plugins/mcpparser/plugin_test.go @@ -41,9 +41,6 @@ func TestMCPParser_Capabilities(t *testing.T) { if !caps.ReadsBody { t.Error("ReadsBody should be true") } - if len(caps.Writes) != 1 || caps.Writes[0] != "mcp" { - t.Errorf("Writes = %v, want [mcp]", caps.Writes) - } } func TestMCPParser_ToolCall(t *testing.T) { diff --git a/authbridge/authlib/plugins/plugintesting/plugintesting.go b/authbridge/authlib/plugins/plugintesting/plugintesting.go index 5f846e5d2..ea2527b2b 100644 --- a/authbridge/authlib/plugins/plugintesting/plugintesting.go +++ b/authbridge/authlib/plugins/plugintesting/plugintesting.go @@ -46,7 +46,7 @@ func NewJWTValidation(a *auth.Auth, audienceFromHost bool) *JWTValidationStub { func (p *JWTValidationStub) Name() string { return "jwt-validation" } func (p *JWTValidationStub) Capabilities() pipeline.PluginCapabilities { - return pipeline.PluginCapabilities{Writes: []string{"security"}} + return pipeline.PluginCapabilities{} } func (p *JWTValidationStub) OnRequest(ctx context.Context, pctx *pipeline.Context) pipeline.Action { diff --git a/authbridge/authlib/plugins/registry.go b/authbridge/authlib/plugins/registry.go index d9421aad6..a69801a9e 100644 --- a/authbridge/authlib/plugins/registry.go +++ b/authbridge/authlib/plugins/registry.go @@ -185,14 +185,9 @@ func cloneCatalog(in []CatalogEntry) []CatalogEntry { Capabilities: pipeline.PluginCapabilities{ ReadsBody: caps.ReadsBody, WritesBody: caps.WritesBody, - BodyAccess: caps.BodyAccess, Description: caps.Description, - Writes: append([]string(nil), caps.Writes...), - Reads: append([]string(nil), caps.Reads...), Requires: append([]string(nil), caps.Requires...), RequiresAny: append([]string(nil), caps.RequiresAny...), - After: append([]string(nil), caps.After...), - Claims: append([]string(nil), caps.Claims...), }, Fields: cloneFieldSchemas(in[i].Fields), } @@ -340,11 +335,11 @@ func BuildWithSPIFFE(entries []config.PluginEntry, p *spiffe.Provider, opts ...p return pipeline.New(ps, opts...) } -// validateRelationships checks every plugin's Requires / RequiresAny / -// After / Claims declarations against the chain it's about to run in. -// Collects all errors across the chain into one joined error rather -// than short-circuiting on the first — friendlier for operators -// iterating on a freshly-edited YAML. +// validateRelationships checks every plugin's Requires / RequiresAny +// declarations against the chain it's about to run in. Collects all +// errors across the chain into one joined error rather than +// short-circuiting on the first — friendlier for operators iterating on +// a freshly-edited YAML. // // Semantics: // @@ -353,15 +348,11 @@ func BuildWithSPIFFE(entries []config.PluginEntry, p *spiffe.Provider, opts ...p // - RequiresAny: at least one named plugin must appear at a lower // index. Any named plugin that IS present must also be at a // lower index. -// - After: if a named plugin is present, it must appear at a lower -// index. Silent if the named plugin is absent. -// - Claims: at most one plugin per unique claim string across the -// entire chain. // -// Each rule loop uses the per-plugin Name() as the identity key. Case- -// sensitive (Go default). If a plugin name is duplicated in a chain -// (rare — requires config.PluginEntry.ID differentiation), the -// earliest-occurrence index is authoritative for position checks. +// Each rule uses the per-plugin Name() as the identity key (case-sensitive). +// If a plugin name is duplicated in a chain (rare — requires +// config.PluginEntry.ID differentiation), the earliest-occurrence index +// is authoritative for position checks. func validateRelationships(ps []pipeline.Plugin) error { if len(ps) == 0 { return nil @@ -403,7 +394,6 @@ func validateRelationships(ps []pipeline.Plugin) error { continue } if j >= i { - // Present but misordered — report per-offender. errs = append(errs, fmt.Sprintf( "plugin %q lists %q under RequiresAny; %q must appear earlier (found at position %d, this plugin is at %d)", p.Name(), req, req, j, i)) @@ -417,34 +407,6 @@ func validateRelationships(ps []pipeline.Plugin) error { p.Name(), caps.RequiresAny)) } } - - // After — soft ordering. - for _, name := range caps.After { - j, present := positions[name] - if !present { - continue - } - if j >= i { - errs = append(errs, fmt.Sprintf( - "plugin %q declares After %q, but %q appears at position %d (this plugin is at %d); reorder so %q runs first", - p.Name(), name, name, j, i, name)) - } - } - } - - // Claims — chain-wide aggregation. - claimOwner := make(map[string]string, len(ps)) - for _, p := range ps { - caps := p.Capabilities().Normalize() - for _, claim := range caps.Claims { - if existing, taken := claimOwner[claim]; taken && existing != p.Name() { - errs = append(errs, fmt.Sprintf( - "plugins %q and %q both claim %q; configure only one of them on this chain", - existing, p.Name(), claim)) - continue - } - claimOwner[claim] = p.Name() - } } if len(errs) == 0 { diff --git a/authbridge/authlib/plugins/registry_test.go b/authbridge/authlib/plugins/registry_test.go index 20b82d286..c203be3c2 100644 --- a/authbridge/authlib/plugins/registry_test.go +++ b/authbridge/authlib/plugins/registry_test.go @@ -259,141 +259,18 @@ func TestValidateRelationships_RequiresAny(t *testing.T) { } } -// TestValidateRelationships_After covers the soft-ordering rule: -// silent when named plugin is absent, error when present but later. -func TestValidateRelationships_After(t *testing.T) { - tests := []struct { - name string - plugins []pipeline.Plugin - wantErr bool - wantInMsg []string - }{ - { - name: "named plugin absent — no constraint", - plugins: []pipeline.Plugin{ - mkRelPlugin("request-counter", pipeline.PluginCapabilities{ - After: []string{"mcp-parser"}, - }), - }, - wantErr: false, - }, - { - name: "named plugin present earlier — ok", - plugins: []pipeline.Plugin{ - mkRelPlugin("mcp-parser", pipeline.PluginCapabilities{}), - mkRelPlugin("request-counter", pipeline.PluginCapabilities{ - After: []string{"mcp-parser"}, - }), - }, - wantErr: false, - }, - { - name: "named plugin present but later — error says reorder", - plugins: []pipeline.Plugin{ - mkRelPlugin("request-counter", pipeline.PluginCapabilities{ - After: []string{"mcp-parser"}, - }), - mkRelPlugin("mcp-parser", pipeline.PluginCapabilities{}), - }, - wantErr: true, - wantInMsg: []string{"request-counter", "After", "mcp-parser", "reorder"}, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - err := validateRelationships(tc.plugins) - if (err != nil) != tc.wantErr { - t.Fatalf("err = %v, wantErr = %v", err, tc.wantErr) - } - if err != nil { - msg := err.Error() - for _, want := range tc.wantInMsg { - if !containsSubstring(msg, want) { - t.Errorf("error %q missing %q", msg, want) - } - } - } - }) - } -} - -// TestValidateRelationships_Claims covers the mutual-exclusion rule: -// exactly one plugin per claim string per chain. -func TestValidateRelationships_Claims(t *testing.T) { - tests := []struct { - name string - plugins []pipeline.Plugin - wantErr bool - wantInMsg []string - }{ - { - name: "single claimant — ok", - plugins: []pipeline.Plugin{ - mkRelPlugin("token-exchange", pipeline.PluginCapabilities{ - Claims: []string{"authorization_header"}, - }), - }, - wantErr: false, - }, - { - name: "distinct claims on different plugins — ok", - plugins: []pipeline.Plugin{ - mkRelPlugin("token-exchange", pipeline.PluginCapabilities{ - Claims: []string{"authorization_header"}, - }), - mkRelPlugin("jwt-validation", pipeline.PluginCapabilities{ - Claims: []string{"identity_resolution"}, - }), - }, - wantErr: false, - }, - { - name: "two plugins claim the same string — error names both", - plugins: []pipeline.Plugin{ - mkRelPlugin("token-exchange", pipeline.PluginCapabilities{ - Claims: []string{"authorization_header"}, - }), - mkRelPlugin("token-broker", pipeline.PluginCapabilities{ - Claims: []string{"authorization_header"}, - }), - }, - wantErr: true, - wantInMsg: []string{"token-exchange", "token-broker", "authorization_header", "configure only one"}, - }, - } - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - err := validateRelationships(tc.plugins) - if (err != nil) != tc.wantErr { - t.Fatalf("err = %v, wantErr = %v", err, tc.wantErr) - } - if err != nil { - msg := err.Error() - for _, want := range tc.wantInMsg { - if !containsSubstring(msg, want) { - t.Errorf("error %q missing %q", msg, want) - } - } - } - }) - } -} - // TestValidateRelationships_CollectsAllErrors verifies the collector // policy: a chain with multiple problems reports them all in one // error, rather than short-circuiting on the first. Operators iterate // on a single YAML fix rather than a sequence of startups. func TestValidateRelationships_CollectsAllErrors(t *testing.T) { plugins := []pipeline.Plugin{ - mkRelPlugin("a-claims-x", pipeline.PluginCapabilities{ - Claims: []string{"x"}, - }), - mkRelPlugin("b-claims-x", pipeline.PluginCapabilities{ - Claims: []string{"x"}, // conflicts with a-claims-x - }), - mkRelPlugin("c-requires-missing", pipeline.PluginCapabilities{ + mkRelPlugin("a-requires-missing", pipeline.PluginCapabilities{ Requires: []string{"does-not-exist"}, }), + mkRelPlugin("b-requires-any-missing", pipeline.PluginCapabilities{ + RequiresAny: []string{"also-missing"}, + }), } err := validateRelationships(plugins) if err == nil { @@ -401,10 +278,10 @@ func TestValidateRelationships_CollectsAllErrors(t *testing.T) { } msg := err.Error() for _, want := range []string{ - "a-claims-x", - "b-claims-x", - "c-requires-missing", + "a-requires-missing", "does-not-exist", + "b-requires-any-missing", + "also-missing", } { if !containsSubstring(msg, want) { t.Errorf("error message should mention %q: %s", want, msg) @@ -629,7 +506,6 @@ func TestCatalog_IncludesRegisteredPlugins(t *testing.T) { name: a, caps: pipeline.PluginCapabilities{ Description: "A plugin", - Writes: []string{"out-a"}, }, } }) @@ -639,7 +515,6 @@ func TestCatalog_IncludesRegisteredPlugins(t *testing.T) { name: b, caps: pipeline.PluginCapabilities{ Description: "B plugin", - Reads: []string{"out-a"}, Requires: []string{a}, }, } @@ -722,7 +597,7 @@ func TestCatalog_ReturnsDefensiveCopy(t *testing.T) { const name = "test-defensive-copy" RegisterPlugin(name, func() pipeline.Plugin { return &relPlugin{name: name, caps: pipeline.PluginCapabilities{ - Writes: []string{"slot-a"}, + Requires: []string{"dep-a"}, }} }) t.Cleanup(func() { UnregisterPlugin(name) }) @@ -740,8 +615,8 @@ func TestCatalog_ReturnsDefensiveCopy(t *testing.T) { t.Fatal("seeded plugin missing from Catalog") } - // Mutate the returned slice and the nested Writes slice. - first[idx].Capabilities.Writes[0] = "MUTATED" + // Mutate the returned slice and the nested Requires slice. + first[idx].Capabilities.Requires[0] = "MUTATED" first[idx].Capabilities.Description = "MUTATED" // A second call must still see the original values. @@ -756,8 +631,8 @@ func TestCatalog_ReturnsDefensiveCopy(t *testing.T) { if idx2 < 0 { t.Fatal("seeded plugin missing from second Catalog call") } - if got := second[idx2].Capabilities.Writes[0]; got != "slot-a" { - t.Errorf("cache tainted by caller mutation: Writes[0] = %q, want slot-a", got) + if got := second[idx2].Capabilities.Requires[0]; got != "dep-a" { + t.Errorf("cache tainted by caller mutation: Requires[0] = %q, want dep-a", got) } if got := second[idx2].Capabilities.Description; got != "" { t.Errorf("cache tainted by caller mutation: Description = %q, want empty", got) diff --git a/authbridge/authlib/sessionapi/catalog_adapter.go b/authbridge/authlib/sessionapi/catalog_adapter.go index 17f4d6845..4f45177ad 100644 --- a/authbridge/authlib/sessionapi/catalog_adapter.go +++ b/authbridge/authlib/sessionapi/catalog_adapter.go @@ -27,12 +27,8 @@ func PluginsCatalog() []CatalogEntry { out[i] = CatalogEntry{ Name: e.Name, ReadsBody: n.ReadsBody, - Writes: n.Writes, - Reads: n.Reads, Requires: n.Requires, RequiresAny: n.RequiresAny, - After: n.After, - Claims: n.Claims, Description: n.Description, Fields: convertFieldSchemas(e.Fields), } diff --git a/authbridge/authlib/sessionapi/server.go b/authbridge/authlib/sessionapi/server.go index 6d18844a0..78ea44b01 100644 --- a/authbridge/authlib/sessionapi/server.go +++ b/authbridge/authlib/sessionapi/server.go @@ -55,12 +55,8 @@ type CatalogEntry struct { Name string `json:"name"` Direction string `json:"direction,omitempty"` ReadsBody bool `json:"readsBody,omitempty"` - Writes []string `json:"writes,omitempty"` - Reads []string `json:"reads,omitempty"` Requires []string `json:"requires,omitempty"` RequiresAny []string `json:"requiresAny,omitempty"` - After []string `json:"after,omitempty"` - Claims []string `json:"claims,omitempty"` Description string `json:"description,omitempty"` Fields []FieldSchemaEntry `json:"fields,omitempty"` } @@ -158,22 +154,18 @@ func (s *Server) handleHealthz(w http.ResponseWriter, _ *http.Request) { // pipelinePluginView is the wire shape for one plugin in /v1/pipeline. // -// The capability fields below (Reads/Writes/Requires/RequiresAny/After/ -// Claims/Description) are static type-level metadata: same for every -// instance produced by a given factory. abctl uses them to render the -// plugin-detail pane and to compute the "deps satisfied" indicator on -// the Pipeline pane without needing a separate /v1/plugins call. +// Capability fields (Requires/RequiresAny/Description) are static +// type-level metadata: same for every instance produced by a given +// factory. abctl uses them to render the plugin-detail pane and to +// compute the "deps satisfied" indicator on the Pipeline pane without +// needing a separate /v1/plugins call. type pipelinePluginView struct { Name string `json:"name"` Direction string `json:"direction"` Position int `json:"position"` // 1-based order within its direction - BodyAccess bool `json:"bodyAccess"` - Writes []string `json:"writes,omitempty"` - Reads []string `json:"reads,omitempty"` + ReadsBody bool `json:"readsBody"` Requires []string `json:"requires,omitempty"` RequiresAny []string `json:"requiresAny,omitempty"` - After []string `json:"after,omitempty"` - Claims []string `json:"claims,omitempty"` Description string `json:"description,omitempty"` Config json.RawMessage `json:"config,omitempty"` } @@ -230,19 +222,12 @@ func describePipeline(h *pipeline.Holder, direction string) []pipelinePluginView for i, pl := range plugins { caps := pl.Capabilities().Normalize() view := pipelinePluginView{ - Name: pl.Name(), - Direction: direction, - Position: i + 1, - // Normalize folds BodyAccess (deprecated) into ReadsBody; - // emit ReadsBody as the wire's BodyAccess field for backward - // compatibility with abctl < the catalog PR. - BodyAccess: caps.ReadsBody, - Writes: caps.Writes, - Reads: caps.Reads, + Name: pl.Name(), + Direction: direction, + Position: i + 1, + ReadsBody: caps.ReadsBody, Requires: caps.Requires, RequiresAny: caps.RequiresAny, - After: caps.After, - Claims: caps.Claims, Description: caps.Description, } // Surface raw config when the plugin was wrapped by the registry. diff --git a/authbridge/authlib/sessionapi/server_test.go b/authbridge/authlib/sessionapi/server_test.go index 35ba20b83..178e6b293 100644 --- a/authbridge/authlib/sessionapi/server_test.go +++ b/authbridge/authlib/sessionapi/server_test.go @@ -288,14 +288,14 @@ func (f *fakePlugin) OnResponse(_ context.Context, _ *pipeline.Context) pipeline func TestHandlePipeline(t *testing.T) { inbound, err := pipeline.New([]pipeline.Plugin{ &fakePlugin{name: "jwt-validation"}, - &fakePlugin{name: "a2a-parser", caps: pipeline.PluginCapabilities{Writes: []string{"a2a"}, BodyAccess: true}}, + &fakePlugin{name: "a2a-parser", caps: pipeline.PluginCapabilities{ReadsBody: true}}, }) if err != nil { t.Fatal(err) } outbound, err := pipeline.New([]pipeline.Plugin{ &fakePlugin{name: "token-exchange"}, - &fakePlugin{name: "mcp-parser", caps: pipeline.PluginCapabilities{Writes: []string{"mcp"}, BodyAccess: true}}, + &fakePlugin{name: "mcp-parser", caps: pipeline.PluginCapabilities{ReadsBody: true}}, }) if err != nil { t.Fatal(err) @@ -329,8 +329,8 @@ func TestHandlePipeline(t *testing.T) { if body.Inbound[0].Name != "jwt-validation" || body.Inbound[0].Position != 1 { t.Errorf("inbound[0] = %+v", body.Inbound[0]) } - if !body.Inbound[1].BodyAccess || len(body.Inbound[1].Writes) == 0 || body.Inbound[1].Writes[0] != "a2a" { - t.Errorf("inbound[1] = %+v", body.Inbound[1]) + if !body.Inbound[1].ReadsBody { + t.Errorf("inbound[1] should have ReadsBody=true, got %+v", body.Inbound[1]) } if body.Outbound[1].Direction != "outbound" { t.Errorf("outbound direction = %q, want outbound", body.Outbound[1].Direction) @@ -612,16 +612,13 @@ func TestHandleGet_SerializesPluginsMap(t *testing.T) { } } -// TestHandlePipelineSurfacesCapabilityMetadata verifies the new -// metadata fields (Requires/RequiresAny/After/Claims/Description) -// flow through to /v1/pipeline and are omitted when empty. +// TestHandlePipelineSurfacesCapabilityMetadata verifies capability metadata +// (Requires/RequiresAny/Description) flows through to /v1/pipeline and is +// omitted when empty. func TestHandlePipelineSurfacesCapabilityMetadata(t *testing.T) { rich := pipeline.PluginCapabilities{ - Writes: []string{"mcp"}, Requires: []string{"a2a-parser"}, RequiresAny: []string{"jwt-validation", "token-broker"}, - After: []string{"mcp-parser"}, - Claims: []string{"authorization-header"}, Description: "Test plugin description", } inbound, err := pipeline.New([]pipeline.Plugin{ @@ -665,15 +662,9 @@ func TestHandlePipelineSurfacesCapabilityMetadata(t *testing.T) { if reqA, _ := got["requiresAny"].([]any); len(reqA) != 2 { t.Errorf("requiresAny = %v", got["requiresAny"]) } - if a, _ := got["after"].([]any); len(a) != 1 || a[0] != "mcp-parser" { - t.Errorf("after = %v", got["after"]) - } - if c, _ := got["claims"].([]any); len(c) != 1 || c[0] != "authorization-header" { - t.Errorf("claims = %v", got["claims"]) - } bare := raw.Inbound[1] - for _, k := range []string{"requires", "requiresAny", "after", "claims", "description"} { + for _, k := range []string{"requires", "requiresAny", "description"} { if _, present := bare[k]; present { t.Errorf("bare plugin should omit %q, got %v", k, bare[k]) } @@ -683,7 +674,7 @@ func TestHandlePipelineSurfacesCapabilityMetadata(t *testing.T) { func TestHandlePluginCatalog_ListsRegisteredPlugins(t *testing.T) { stub := func() []CatalogEntry { return []CatalogEntry{ - {Name: "alpha", Description: "First plugin", Writes: []string{"x"}}, + {Name: "alpha", Description: "First plugin"}, {Name: "beta", Requires: []string{"alpha"}}, } } From a5a91a5d54436b40a50c08359a479bc48aeed0c1 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Mon, 1 Jun 2026 20:00:10 -0400 Subject: [PATCH 2/3] fix(abctl): sync wire format and remove dead capability fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit renamed bodyAccess → readsBody on the /v1/pipeline wire and dropped writes/reads/after/claims. This commit brings abctl into sync: - PipelinePlugin: BodyAccess → ReadsBody (json:"readsBody"); drop Writes, Reads, After, Claims - PluginCatalogEntry: drop Writes, Reads, After, Claims - pipeline_pane: remove WRITES column; fix body indicator field name - plugin_detail_pane: remove Writes/Reads/After/Claims render sections - deps: remove afterStatus(); update pluginDepsAllSatisfied/HasAnyDeps - catalog_pane: remove dead field mappings - edit/validate: remove After and Claims client-side validation (catalog no longer emits those fields) - All associated tests updated Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/cmd/abctl/apiclient/client.go | 14 +----- authbridge/cmd/abctl/apiclient/client_test.go | 23 +++------ authbridge/cmd/abctl/edit/validate.go | 49 ++----------------- authbridge/cmd/abctl/edit/validate_test.go | 44 +---------------- authbridge/cmd/abctl/tui/catalog_pane.go | 6 +-- authbridge/cmd/abctl/tui/catalog_pane_test.go | 4 -- authbridge/cmd/abctl/tui/deps.go | 40 ++------------- authbridge/cmd/abctl/tui/deps_test.go | 23 +-------- authbridge/cmd/abctl/tui/pipeline_pane.go | 15 ++---- .../cmd/abctl/tui/plugin_detail_pane.go | 25 +--------- .../cmd/abctl/tui/plugin_detail_pane_test.go | 31 ------------ 11 files changed, 25 insertions(+), 249 deletions(-) diff --git a/authbridge/cmd/abctl/apiclient/client.go b/authbridge/cmd/abctl/apiclient/client.go index 06b8eab0c..4c2fe862f 100644 --- a/authbridge/cmd/abctl/apiclient/client.go +++ b/authbridge/cmd/abctl/apiclient/client.go @@ -86,13 +86,9 @@ type PipelinePlugin struct { Name string `json:"name"` Direction string `json:"direction"` Position int `json:"position"` - BodyAccess bool `json:"bodyAccess"` - Writes []string `json:"writes"` - Reads []string `json:"reads"` + ReadsBody bool `json:"readsBody"` Requires []string `json:"requires,omitempty"` RequiresAny []string `json:"requiresAny,omitempty"` - After []string `json:"after,omitempty"` - Claims []string `json:"claims,omitempty"` Description string `json:"description,omitempty"` Config json.RawMessage `json:"config,omitempty"` } @@ -114,20 +110,12 @@ type PluginCatalog struct { // PluginCatalogEntry mirrors the server's sessionapi.CatalogEntry. // Describes a registered plugin's static type-level metadata; the // catalog includes plugins not currently in the active pipeline. -// -// readsBody is the modern field name (matches pipeline.PluginCapabilities -// post-Normalize); the older bodyAccess alias is intentionally NOT -// emitted on this new wire shape. type PluginCatalogEntry struct { Name string `json:"name"` Direction string `json:"direction,omitempty"` ReadsBody bool `json:"readsBody,omitempty"` - Writes []string `json:"writes,omitempty"` - Reads []string `json:"reads,omitempty"` Requires []string `json:"requires,omitempty"` RequiresAny []string `json:"requiresAny,omitempty"` - After []string `json:"after,omitempty"` - Claims []string `json:"claims,omitempty"` Description string `json:"description,omitempty"` Fields []PluginFieldEntry `json:"fields,omitempty"` } diff --git a/authbridge/cmd/abctl/apiclient/client_test.go b/authbridge/cmd/abctl/apiclient/client_test.go index 76d78dc70..774e3f0e2 100644 --- a/authbridge/cmd/abctl/apiclient/client_test.go +++ b/authbridge/cmd/abctl/apiclient/client_test.go @@ -85,8 +85,8 @@ func TestGetPipeline(t *testing.T) { } w.Header().Set("Content-Type", "application/json") _, _ = w.Write([]byte(`{ - "inbound": [{"name":"jwt-validation","direction":"inbound","position":1,"bodyAccess":false}, - {"name":"a2a-parser","direction":"inbound","position":2,"bodyAccess":true,"writes":["a2a"]}], + "inbound": [{"name":"jwt-validation","direction":"inbound","position":1,"readsBody":false}, + {"name":"a2a-parser","direction":"inbound","position":2,"readsBody":true}], "outbound": [{"name":"token-exchange","direction":"outbound","position":1}] }`)) })) @@ -100,21 +100,18 @@ func TestGetPipeline(t *testing.T) { if len(got.Inbound) != 2 || len(got.Outbound) != 1 { t.Fatalf("got %d inbound / %d outbound, want 2/1", len(got.Inbound), len(got.Outbound)) } - if got.Inbound[1].Name != "a2a-parser" || !got.Inbound[1].BodyAccess { + if got.Inbound[1].Name != "a2a-parser" || !got.Inbound[1].ReadsBody { t.Errorf("inbound[1] = %+v", got.Inbound[1]) } - if len(got.Inbound[1].Writes) != 1 || got.Inbound[1].Writes[0] != "a2a" { - t.Errorf("inbound[1].Writes = %v, want [a2a]", got.Inbound[1].Writes) - } } // TestPipelinePluginDecodesConfig verifies the new Config field on // /v1/pipeline survives JSON round-trip through PipelinePlugin. func TestPipelinePluginDecodesConfig(t *testing.T) { body := `{"inbound":[ - {"name":"with-config","direction":"inbound","position":1,"bodyAccess":false, + {"name":"with-config","direction":"inbound","position":1,"readsBody":false, "config":{"hello":"world"}}, - {"name":"without-config","direction":"inbound","position":2,"bodyAccess":false} + {"name":"without-config","direction":"inbound","position":2,"readsBody":false} ],"outbound":[]}` srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -254,7 +251,7 @@ func TestGetPluginCatalog_DecodesFieldSchemas(t *testing.T) { } } -// TestPipelinePluginDecodesCapabilityMetadata verifies the new +// TestPipelinePluginDecodesCapabilityMetadata verifies the capability // metadata fields decode correctly through the apiclient. func TestPipelinePluginDecodesCapabilityMetadata(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -271,8 +268,6 @@ func TestPipelinePluginDecodesCapabilityMetadata(t *testing.T) { "position": 1, "requires": ["a"], "requiresAny": ["b","c"], - "after": ["d"], - "claims": ["sec"], "description": "Rich plugin" } ], @@ -293,12 +288,6 @@ func TestPipelinePluginDecodesCapabilityMetadata(t *testing.T) { if len(p.RequiresAny) != 2 { t.Errorf("RequiresAny = %v", p.RequiresAny) } - if len(p.After) != 1 || p.After[0] != "d" { - t.Errorf("After = %v", p.After) - } - if len(p.Claims) != 1 || p.Claims[0] != "sec" { - t.Errorf("Claims = %v", p.Claims) - } if p.Description != "Rich plugin" { t.Errorf("Description = %q", p.Description) } diff --git a/authbridge/cmd/abctl/edit/validate.go b/authbridge/cmd/abctl/edit/validate.go index 08bc05b82..7792e4725 100644 --- a/authbridge/cmd/abctl/edit/validate.go +++ b/authbridge/cmd/abctl/edit/validate.go @@ -45,14 +45,10 @@ type pipelineRoot struct { Pipeline pipelineDoc `yaml:"pipeline"` } -// ValidatePipeline parses subtree YAML and checks Requires / -// RequiresAny / After / Claims against the catalog. Catalog comes -// from /v1/plugins; passing nil disables validation (no errors -// returned). Unknown plugin names produce errors so a typo gets -// caught before apply. -// -// The Claims check enforces cross-plugin uniqueness within a single -// chain — same as plugins.validateRelationships. +// ValidatePipeline parses subtree YAML and checks Requires / RequiresAny +// against the catalog. Catalog comes from /v1/plugins; passing nil disables +// validation (no errors returned). Unknown plugin names produce errors so +// a typo gets caught before apply. // // Returns nil when all checks pass. func ValidatePipeline(subtree []byte, catalog []apiclient.PluginCatalogEntry) []ValidationError { @@ -76,8 +72,7 @@ func ValidatePipeline(subtree []byte, catalog []apiclient.PluginCatalogEntry) [] return errs } -// validateChain runs the Requires/RequiresAny/After/Claims/unknown-name -// checks for one direction. +// validateChain runs the Requires/RequiresAny/unknown-name checks for one direction. func validateChain(direction string, chain pipelineChain, byName map[string]apiclient.PluginCatalogEntry) []ValidationError { var errs []ValidationError // Track positions of each name for ordering checks. Using lowest @@ -88,8 +83,6 @@ func validateChain(direction string, chain pipelineChain, byName map[string]apic positions[p.Name] = i + 1 } } - // Track claims; first-declarer wins for the diagnostic. - claimedBy := map[string]string{} for i, p := range chain.Plugins { pos := i + 1 @@ -166,38 +159,6 @@ func validateChain(direction string, chain pipelineChain, byName map[string]apic } } - // After: present-at-or-after-this-position is a misorder. Matches - // the framework's validateRelationships rule (j >= i, not j > i) - // so a plugin listing itself in After or having a duplicate at - // the same index is flagged identically by abctl and the - // framework. - for _, name := range entry.After { - rp, present := positions[name] - if present && rp >= pos { - errs = append(errs, ValidationError{ - Direction: direction, - PluginName: p.Name, - Position: pos, - Message: fmt.Sprintf("After %q expects it earlier; it's at position %d (must be < %d)", - name, rp, pos), - }) - } - } - - // Claims: at most one declarer per claim string per chain. - for _, claim := range entry.Claims { - if other, taken := claimedBy[claim]; taken && other != p.Name { - errs = append(errs, ValidationError{ - Direction: direction, - PluginName: p.Name, - Position: pos, - Message: fmt.Sprintf("Claim %q already declared by %q in this chain", - claim, other), - }) - } else { - claimedBy[claim] = p.Name - } - } } return errs } diff --git a/authbridge/cmd/abctl/edit/validate_test.go b/authbridge/cmd/abctl/edit/validate_test.go index e97a4f32d..125ca3f1a 100644 --- a/authbridge/cmd/abctl/edit/validate_test.go +++ b/authbridge/cmd/abctl/edit/validate_test.go @@ -12,9 +12,7 @@ func validateFixtureCatalog() []apiclient.PluginCatalogEntry { {Name: "jwt-validation", Description: "Inbound JWT"}, {Name: "a2a-parser", Description: "Parser"}, {Name: "mcp-parser", Description: "MCP parser"}, - {Name: "ibac", Description: "IBAC", Requires: []string{"mcp-parser"}, After: []string{"a2a-parser"}}, - {Name: "claim-a", Claims: []string{"authorization-header"}}, - {Name: "claim-b", Claims: []string{"authorization-header"}}, + {Name: "ibac", Description: "IBAC", Requires: []string{"mcp-parser"}}, } } @@ -85,27 +83,6 @@ func TestValidatePipeline_MisorderedRequires(t *testing.T) { } } -func TestValidatePipeline_AfterMisorder(t *testing.T) { - yaml := `pipeline: - outbound: - plugins: - - name: ibac - - name: a2a-parser - - name: mcp-parser -` - errs := ValidatePipeline([]byte(yaml), validateFixtureCatalog()) - // ibac.After=[a2a-parser]; a2a-parser is at position 2 > 1. - found := false - for _, e := range errs { - if e.PluginName == "ibac" && strings.Contains(e.Message, "After \"a2a-parser\"") { - found = true - } - } - if !found { - t.Fatalf("expected After-misorder for ibac; got %+v", errs) - } -} - func TestValidatePipeline_UnknownPlugin(t *testing.T) { yaml := `pipeline: inbound: @@ -118,25 +95,6 @@ func TestValidatePipeline_UnknownPlugin(t *testing.T) { } } -func TestValidatePipeline_ClaimsConflict(t *testing.T) { - yaml := `pipeline: - outbound: - plugins: - - name: claim-a - - name: claim-b -` - errs := ValidatePipeline([]byte(yaml), validateFixtureCatalog()) - found := false - for _, e := range errs { - if strings.Contains(e.Message, "already declared") { - found = true - } - } - if !found { - t.Fatalf("expected Claims conflict; got %+v", errs) - } -} - func TestValidatePipeline_NilCatalogSkips(t *testing.T) { yaml := `pipeline: outbound: diff --git a/authbridge/cmd/abctl/tui/catalog_pane.go b/authbridge/cmd/abctl/tui/catalog_pane.go index b6c9b331f..e74c05aeb 100644 --- a/authbridge/cmd/abctl/tui/catalog_pane.go +++ b/authbridge/cmd/abctl/tui/catalog_pane.go @@ -68,13 +68,9 @@ func (m *model) selectedCatalogEntry() *apiclient.PipelinePlugin { if e.Name == name { p := apiclient.PipelinePlugin{ Name: e.Name, - BodyAccess: e.ReadsBody, - Writes: e.Writes, - Reads: e.Reads, + ReadsBody: e.ReadsBody, Requires: e.Requires, RequiresAny: e.RequiresAny, - After: e.After, - Claims: e.Claims, Description: e.Description, } return &p diff --git a/authbridge/cmd/abctl/tui/catalog_pane_test.go b/authbridge/cmd/abctl/tui/catalog_pane_test.go index c618f476b..cfd219d1d 100644 --- a/authbridge/cmd/abctl/tui/catalog_pane_test.go +++ b/authbridge/cmd/abctl/tui/catalog_pane_test.go @@ -68,7 +68,6 @@ func TestSelectedCatalogEntry_AsPipelinePlugin(t *testing.T) { Name: "ibac", Description: "Judge", Requires: []string{"mcp-parser"}, - After: []string{"a2a-parser"}, }, }, } @@ -83,9 +82,6 @@ func TestSelectedCatalogEntry_AsPipelinePlugin(t *testing.T) { if len(got.Requires) != 1 || got.Requires[0] != "mcp-parser" { t.Errorf("Requires = %v", got.Requires) } - if len(got.After) != 1 { - t.Errorf("After = %v", got.After) - } // Direction and Position deliberately blank for catalog entries — // showPluginDetail elides them. if got.Direction != "" || got.Position != 0 { diff --git a/authbridge/cmd/abctl/tui/deps.go b/authbridge/cmd/abctl/tui/deps.go index 3d9a9fa52..21c97f3b2 100644 --- a/authbridge/cmd/abctl/tui/deps.go +++ b/authbridge/cmd/abctl/tui/deps.go @@ -99,54 +99,22 @@ func requiresAnyStatus(p *apiclient.PipelinePlugin, chain []apiclient.PipelinePl return out } -// afterStatus returns one depCheck per entry in p.After. Satisfied -// when the named plugin is absent (After is a soft hint) or present -// at a lower position. Misorder (present at higher position) is the -// only failure case. -func afterStatus(p *apiclient.PipelinePlugin, chain []apiclient.PipelinePlugin) []depCheck { - out := make([]depCheck, 0, len(p.After)) - for _, name := range p.After { - c := depCheck{Name: name, Satisfied: true} - for _, q := range chain { - if q.Name == name { - if q.Position < p.Position { - c.UpstreamPosition = q.Position - } else if q.Position > p.Position { - c.Satisfied = false - } - break - } - } - out = append(out, c) - } - return out -} - -// pluginDepsAllSatisfied returns true iff Requires, RequiresAny, and -// After are all OK for p in chain. Drives the Pipeline pane's per-row -// ✓/✗ indicator. Plugins with no declared deps are always ✓. +// pluginDepsAllSatisfied returns true iff Requires and RequiresAny are +// all OK for p in chain. Drives the Pipeline pane's per-row ✓/✗ indicator. func pluginDepsAllSatisfied(p *apiclient.PipelinePlugin, chain []apiclient.PipelinePlugin) bool { for _, c := range requiresStatus(p, chain) { if !c.Satisfied { return false } } - if !requiresAnyOK(p, chain) { - return false - } - for _, c := range afterStatus(p, chain) { - if !c.Satisfied { - return false - } - } - return true + return requiresAnyOK(p, chain) } // pluginHasAnyDeps reports whether p declares any dependency that the // indicator can render. Plugins without any declarations get a blank // indicator (no false-positive ✓). func pluginHasAnyDeps(p *apiclient.PipelinePlugin) bool { - return len(p.Requires) > 0 || len(p.RequiresAny) > 0 || len(p.After) > 0 + return len(p.Requires) > 0 || len(p.RequiresAny) > 0 } // formatDepCheck returns a one-line description of a dependency check. diff --git a/authbridge/cmd/abctl/tui/deps_test.go b/authbridge/cmd/abctl/tui/deps_test.go index 394e183a9..9e4630176 100644 --- a/authbridge/cmd/abctl/tui/deps_test.go +++ b/authbridge/cmd/abctl/tui/deps_test.go @@ -56,25 +56,6 @@ func TestPluginDepsAllSatisfied_RequiresAnyAllMissing(t *testing.T) { } } -func TestPluginDepsAllSatisfied_AfterAbsentIsOK(t *testing.T) { - chain := []apiclient.PipelinePlugin{ - {Name: "ibac", Direction: "outbound", Position: 1, After: []string{"mcp-parser"}}, - } - if !pluginDepsAllSatisfied(&chain[0], chain) { - t.Fatal("After=['mcp-parser'] should be satisfied when mcp-parser is absent (soft hint)") - } -} - -func TestPluginDepsAllSatisfied_AfterMisordered(t *testing.T) { - chain := []apiclient.PipelinePlugin{ - {Name: "ibac", Direction: "outbound", Position: 1, After: []string{"mcp-parser"}}, - {Name: "mcp-parser", Direction: "outbound", Position: 2}, - } - if pluginDepsAllSatisfied(&chain[0], chain) { - t.Fatal("After=['mcp-parser'] should NOT be satisfied when mcp-parser is downstream") - } -} - func TestPluginHasAnyDeps(t *testing.T) { if pluginHasAnyDeps(&apiclient.PipelinePlugin{Name: "x"}) { t.Fatal("plugin with no deps should report false") @@ -82,8 +63,8 @@ func TestPluginHasAnyDeps(t *testing.T) { if !pluginHasAnyDeps(&apiclient.PipelinePlugin{Name: "x", Requires: []string{"a"}}) { t.Fatal("plugin with Requires should report true") } - if !pluginHasAnyDeps(&apiclient.PipelinePlugin{Name: "x", After: []string{"a"}}) { - t.Fatal("plugin with After should report true") + if !pluginHasAnyDeps(&apiclient.PipelinePlugin{Name: "x", RequiresAny: []string{"a"}}) { + t.Fatal("plugin with RequiresAny should report true") } } diff --git a/authbridge/cmd/abctl/tui/pipeline_pane.go b/authbridge/cmd/abctl/tui/pipeline_pane.go index 4bfb27d4f..10c08b0a1 100644 --- a/authbridge/cmd/abctl/tui/pipeline_pane.go +++ b/authbridge/cmd/abctl/tui/pipeline_pane.go @@ -2,7 +2,6 @@ package tui import ( "fmt" - "strings" "github.com/charmbracelet/bubbles/table" @@ -19,7 +18,6 @@ func newPipelineTable() table.Model { {Title: "DIRECTION", Width: 10}, {Title: "PLUGIN", Width: 22}, {Title: "DEPS", Width: 5}, - {Title: "WRITES", Width: 18}, {Title: "BODY", Width: 6}, {Title: "EVENTS", Width: 8}, }), @@ -30,9 +28,7 @@ func newPipelineTable() table.Model { } // rebuildPipelineTable renders the plugin list with a "(app)" divider row -// between inbound and outbound. eventsPerPlugin counts how many events in -// the cached data came from each plugin (by matching the event's written -// extension against the plugin's Writes). +// between inbound and outbound. func (m *model) rebuildPipelineTable() { if m.pipeline == nil { m.pipelineTbl.SetRows(nil) @@ -58,7 +54,7 @@ func (m *model) rebuildPipelineTable() { func pipelineRow(p apiclient.PipelinePlugin, events int, chain []apiclient.PipelinePlugin) table.Row { body := "no" - if p.BodyAccess { + if p.ReadsBody { body = "yes" } eventsStr := "" @@ -66,7 +62,7 @@ func pipelineRow(p apiclient.PipelinePlugin, events int, chain []apiclient.Pipel eventsStr = fmt.Sprintf("%d", events) } // DEPS column: ✓ when all declared dependencies are met, ✗ when any - // fail, blank when the plugin declares no Requires/RequiresAny/After. + // fail, blank when the plugin declares no Requires/RequiresAny. // Blank vs ✓ avoids a misleading "looks fine" mark on plugins that // have nothing to verify in the first place. deps := "" @@ -77,16 +73,11 @@ func pipelineRow(p apiclient.PipelinePlugin, events int, chain []apiclient.Pipel deps = "✗" } } - // Plugin names used to be colored by protocol but bubbles v1's - // runewidth.Truncate miscounts ANSI escape bytes as visible width, - // which truncated the closing \x1b[0m reset for longer names and - // bled color into adjacent cells. Blocked on bubbles v2 upgrade. return table.Row{ fmt.Sprintf("%d", p.Position), p.Direction, p.Name, deps, - strings.Join(p.Writes, ","), body, eventsStr, } diff --git a/authbridge/cmd/abctl/tui/plugin_detail_pane.go b/authbridge/cmd/abctl/tui/plugin_detail_pane.go index dec1896ed..a786b0c1a 100644 --- a/authbridge/cmd/abctl/tui/plugin_detail_pane.go +++ b/authbridge/cmd/abctl/tui/plugin_detail_pane.go @@ -12,7 +12,7 @@ import ( // and human-readable. // // When m.pipeline is non-nil and the plugin's direction is "inbound" or -// "outbound", the Requires/RequiresAny/After sections render with ✓/✗ +// "outbound", the Requires/RequiresAny sections render with ✓/✗ // indicators against the active chain. For catalog-pane invocations // (no live pipeline / direction), those sections render as informational // lists without satisfaction status. @@ -33,14 +33,8 @@ func (m *model) showPluginDetail(p *apiclient.PipelinePlugin) { if p.Position > 0 { fmt.Fprintf(&b, "%s %d\n", styleMuted.Render("Position: "), p.Position) } - if len(p.Writes) > 0 { - fmt.Fprintf(&b, "%s %s\n", styleMuted.Render("Writes: "), strings.Join(p.Writes, ", ")) - } - if len(p.Reads) > 0 { - fmt.Fprintf(&b, "%s %s\n", styleMuted.Render("Reads: "), strings.Join(p.Reads, ", ")) - } body := "no" - if p.BodyAccess { + if p.ReadsBody { body = "yes" } fmt.Fprintf(&b, "%s %s\n", styleMuted.Render("Body: "), body) @@ -69,21 +63,6 @@ func (m *model) showPluginDetail(p *apiclient.PipelinePlugin) { b.WriteString("\n") } } - if len(p.After) > 0 { - fmt.Fprintln(&b) - b.WriteString(styleMuted.Render("After (soft):")) - b.WriteString("\n") - for _, c := range afterStatus(p, chain) { - b.WriteString(" ") - b.WriteString(formatDepCheck(c, chain != nil)) - b.WriteString("\n") - } - } - if len(p.Claims) > 0 { - fmt.Fprintln(&b) - fmt.Fprintf(&b, "%s %s\n", styleMuted.Render("Claims: "), strings.Join(p.Claims, ", ")) - } - fmt.Fprintln(&b) // Always-newline format keeps the visual layout consistent whether // the plugin is Configurable (JSON body, multi-line) or not ("(none)", diff --git a/authbridge/cmd/abctl/tui/plugin_detail_pane_test.go b/authbridge/cmd/abctl/tui/plugin_detail_pane_test.go index e7d54a2ad..3f2890c21 100644 --- a/authbridge/cmd/abctl/tui/plugin_detail_pane_test.go +++ b/authbridge/cmd/abctl/tui/plugin_detail_pane_test.go @@ -19,7 +19,6 @@ func TestShowPluginDetailRendersConfig(t *testing.T) { Name: "jwt-validation", Direction: "inbound", Position: 1, - Writes: []string{"security"}, Config: json.RawMessage(`{"issuer":"http://idp"}`), } m.showPluginDetail(plugin) @@ -88,36 +87,6 @@ func TestShowPluginDetailHandlesMalformedConfig(t *testing.T) { } } -// TestShowPluginDetailRendersRequiresSatisfied renders ibac with -// mcp-parser already at a lower outbound position; the After-section -// indicator should report ✓. -func TestShowPluginDetailRendersRequiresSatisfied(t *testing.T) { - m := newPickerModel(context.Background(), nil, nil) - m.detailVp.Width = 80 - m.detailVp.Height = 30 - m.pipeline = &apiclient.PipelineView{ - Outbound: []apiclient.PipelinePlugin{ - {Name: "mcp-parser", Direction: "outbound", Position: 1}, - {Name: "ibac", Direction: "outbound", Position: 2, After: []string{"mcp-parser"}}, - }, - } - plugin := &m.pipeline.Outbound[1] - m.showPluginDetail(plugin) - view := m.detailVp.View() - if !strings.Contains(view, "After (soft)") { - t.Fatalf("After section missing:\n%s", view) - } - if !strings.Contains(view, "mcp-parser") { - t.Fatalf("After should mention mcp-parser:\n%s", view) - } - if !strings.Contains(view, "position 1") { - t.Fatalf("After should report upstream position 1:\n%s", view) - } -} - -// TestShowPluginDetailRendersRequiresUnmet sets up ibac without -// mcp-parser anywhere; After should still pass (soft hint, absent -// is OK), but the helper render path is exercised. func TestShowPluginDetailRendersDescription(t *testing.T) { m := newPickerModel(context.Background(), nil, nil) m.detailVp.Width = 80 From cb0ccf9521bc93b77e9e315e62021ae90e108db5 Mon Sep 17 00:00:00 2001 From: Hai Huang Date: Mon, 1 Jun 2026 20:09:38 -0400 Subject: [PATCH 3/3] docs(authbridge): remove Reads/Writes/After/Claims from plugin docs Update plugin-reference.md, plugin-tutorial.md, and CLAUDE.md to reflect the simplified PluginCapabilities: - Declaring plugin relationships: struct, table, and subsections for After and Claims removed; now documents only Requires/RequiresAny - Body mutation capability fields: remove Reads, Writes, BodyAccess from the struct example - plugin-tutorial.md gotchas: remove stale note about slot validation (Reads/Writes slot checking was removed from the framework) - CLAUDE.md: update /v1/pipeline and /v1/plugins endpoint wire shapes Assisted-By: Claude (Anthropic AI) Signed-off-by: Hai Huang --- authbridge/CLAUDE.md | 4 +- authbridge/docs/plugin-reference.md | 74 +++++------------------------ authbridge/docs/plugin-tutorial.md | 5 -- 3 files changed, 13 insertions(+), 70 deletions(-) diff --git a/authbridge/CLAUDE.md b/authbridge/CLAUDE.md index ba5376d56..cb67b20e8 100644 --- a/authbridge/CLAUDE.md +++ b/authbridge/CLAUDE.md @@ -408,8 +408,8 @@ When `session.enabled` is true (default) and `listener.session_api_addr` is non- | `GET /v1/sessions` | `application/json` | List active sessions: `{sessions: [{id, createdAt, updatedAt, eventCount, active}]}`. | | `GET /v1/sessions/{id}` | `application/json` | Full snapshot of one session's events. 404 if unknown/expired. | | `GET /v1/events` | `text/event-stream` | SSE stream of new events. Optional `?session=` filters to one session. Heartbeat every 30s. | -| `GET /v1/pipeline` | `application/json` | Active pipeline composition: `{inbound: [...], outbound: [...]}`. Each plugin entry carries `name`, `direction`, `position`, `bodyAccess`, `writes`, `reads`, plus the static metadata (`requires`, `requiresAny`, `after`, `claims`, `description`) and runtime `config` when present. abctl renders this as the Pipeline pane. | -| `GET /v1/plugins` | `application/json` | Catalog of every registered plugin (whether or not in the active pipeline): `{plugins: [{name, requires, requiresAny, after, claims, description, ...}]}`. abctl renders this as the Catalog pane (`P` key). 404s when the binary's session API was constructed without `WithCatalog`. | +| `GET /v1/pipeline` | `application/json` | Active pipeline composition: `{inbound: [...], outbound: [...]}`. Each plugin entry carries `name`, `direction`, `position`, `readsBody`, plus the static metadata (`requires`, `requiresAny`, `description`) and runtime `config` when present. abctl renders this as the Pipeline pane. | +| `GET /v1/plugins` | `application/json` | Catalog of every registered plugin (whether or not in the active pipeline): `{plugins: [{name, requires, requiresAny, description, ...}]}`. abctl renders this as the Catalog pane (`P` key). 404s when the binary's session API was constructed without `WithCatalog`. | | `GET /healthz` | text | Liveness probe. | ### Quick examples diff --git a/authbridge/docs/plugin-reference.md b/authbridge/docs/plugin-reference.md index f430c9b84..88838cffe 100644 --- a/authbridge/docs/plugin-reference.md +++ b/authbridge/docs/plugin-reference.md @@ -189,8 +189,8 @@ canaried; auth gates should stay on `enforce` (the default). ## Declaring plugin relationships -`PluginCapabilities` carries four fields that let a plugin express how it -relates to other plugins in the same chain. All four are checked at +`PluginCapabilities` carries two fields that let a plugin express how it +depends on other plugins in the same chain. Both are checked at `plugins.Build` time (startup and hot-reload), and misconfigurations fail loud before serving traffic. @@ -198,24 +198,20 @@ fail loud before serving traffic. type PluginCapabilities struct { ReadsBody bool WritesBody bool - Reads []string - Writes []string Requires []string // ALL must be present + earlier (hard) RequiresAny []string // AT LEAST ONE must be present + run after it (hard) - After []string // SOFT ordering; silent if absent - Claims []string // <=1 per claim per chain (mutex) + + Description string } ``` -| Field | All present? | At least one? | Silent if absent? | Ordering enforced? | -|---|---|---|---|---| -| `Requires` | ✓ | — | — | ✓ | -| `RequiresAny` | — | ✓ | — | ✓ | -| `After` | — | — | ✓ | ✓ | -| `Claims` | — | — | — | — (mutex) | +| Field | All present? | At least one? | Ordering enforced? | +|---|---|---|---| +| `Requires` | ✓ | — | ✓ | +| `RequiresAny` | — | ✓ | ✓ | -All four are chain-scoped — validation runs within the inbound chain +Both are chain-scoped — validation runs within the inbound chain OR within the outbound chain, independently. Plugin names are case-sensitive and must match the `Name()` returned by the plugin. @@ -255,48 +251,6 @@ func (p *PIIScrubber) Capabilities() pipeline.PluginCapabilities { } ``` -### `After` — soft ordering - -Use for optional ordering relationships: the plugin benefits from a -named plugin running earlier when present, but runs fine without it. -Absent named plugin is not an error. - -```go -// Adds per-protocol labels to request counts if a parser is -// present; falls back to generic labels otherwise. -func (p *RequestCounter) Capabilities() pipeline.PluginCapabilities { - return pipeline.PluginCapabilities{ - After: []string{"a2a-parser", "mcp-parser", "inference-parser"}, - } -} -``` - -### `Claims` — mutual exclusion - -A claim is a semantic resource that exactly one plugin per chain -owns. Two plugins declaring the same claim fail startup. - -Claim strings are arbitrary, but the well-known set lives as Go -constants in `authbridge/authlib/contracts/claims.go` — plugin -authors reference the constants instead of string literals so -typos are compile errors and the canonical set is greppable. - -```go -import "github.com/kagenti/kagenti-extensions/authbridge/authlib/contracts" - -// token-exchange and token-broker both declare this; they can't -// coexist in the same outbound chain. -func (p *TokenExchange) Capabilities() pipeline.PluginCapabilities { - return pipeline.PluginCapabilities{ - Claims: []string{contracts.ClaimAuthorizationHeader}, - } -} -``` - -Third-party plugins may declare arbitrary strings — the framework -enforces uniqueness of whatever it sees, not "must be from the list." -Upstream a new constant in a follow-up PR when a claim stabilizes. - ### Error collection When validation fails the error aggregates every violation in the @@ -725,11 +679,8 @@ before/after (never the raw body). ```go type PluginCapabilities struct { - Reads []string // extension slot names this plugin reads - Writes []string // extension slot names this plugin writes - ReadsBody bool // plugin reads pctx.Body / pctx.ResponseBody - WritesBody bool // plugin may call pctx.SetBody / pctx.SetResponseBody - BodyAccess bool // DEPRECATED alias for ReadsBody; folded by Normalize() + ReadsBody bool // plugin reads pctx.Body / pctx.ResponseBody + WritesBody bool // plugin may call pctx.SetBody / pctx.SetResponseBody } ``` @@ -737,9 +688,6 @@ type PluginCapabilities struct { - `WritesBody`: implies `ReadsBody`. Listener propagates `pctx.SetBody` rewrites to the upstream (and `pctx.SetResponseBody` to the downstream client). -- `BodyAccess`: deprecated. `PluginCapabilities.Normalize()` folds it - into `ReadsBody` for one release of migration grace; new plugins - should never set it. ### Build-time validation (enforced by `pipeline.New`) diff --git a/authbridge/docs/plugin-tutorial.md b/authbridge/docs/plugin-tutorial.md index 394622f8f..544b3e1bc 100644 --- a/authbridge/docs/plugin-tutorial.md +++ b/authbridge/docs/plugin-tutorial.md @@ -382,11 +382,6 @@ All optional. A plugin that doesn't implement them is treated as framework fields after each plugin returns. Recording an invocation from a spawned goroutine attributes it to whichever plugin the pipeline happens to be dispatching at the time — usually garbage. -- **Reads/writes on Extensions slots aren't compile-checked.** The - pipeline's `Capabilities` validation catches "plugin A reads slot X - but no earlier plugin writes X," but typos in string names silently - fall through. Use the constants in `pipeline/extensions.go` when - they exist. - **DisallowUnknownFields or nothing.** Strict decode in Configure is not optional. A misspelled key at startup is always a bug; lenient decode hides it until it misbehaves at 3am.