Skip to content

Commit 276c97d

Browse files
committed
docs(skills): add agent skill files for transit development
Five SKILL.md files covering: envoy-abi-wrapper (canonical, replaces .claude/skills/ stub), transit-e2e-authoring, transit-e2e-runner, transit-example-creator, and transit-unit-testing.
1 parent cdd8142 commit 276c97d

5 files changed

Lines changed: 708 additions & 0 deletions

File tree

Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
---
2+
name: envoy-abi-wrapper
3+
description: Maintain Envoy dynamic module ABI wrappers in transit. Use when changing Cluster Extension or LB Policy APIs, CGO //export wrappers, Envoy callback mappings, async host selection, scheduler/main-thread behavior, or the up/down re-export surface.
4+
---
5+
6+
# Envoy ABI Wrapper - transit
7+
8+
Use this skill for work that touches transit's Envoy dynamic module ABI boundary:
9+
`down/down.go`, `down/abi_impl/cluster.go`, `down/abi_impl/lb_policy.go`,
10+
`down/abi_impl/internal.go`, `up/cluster.go`, or `up/lb.go`.
11+
12+
The core job is to keep the public Go API CGO-free while mapping Envoy's C ABI
13+
precisely inside `down/abi_impl`.
14+
15+
## Current layout
16+
17+
```
18+
down/
19+
down.go public interfaces, registries, aliases, no CGO
20+
abi_impl/
21+
abi.h vendored Envoy dynamic module C ABI
22+
internal.go manager[T], buffer helpers, hostLog
23+
access_logger.go mature template for lifecycle + manager patterns
24+
cluster.go Cluster Extension ABI implementation
25+
lb_policy.go LB Policy ABI implementation
26+
up/
27+
cluster.go public Cluster aliases and RegisterCluster
28+
lb.go public LB Policy aliases and RegisterLBPolicy
29+
up.go HTTP/access-log registration surface
30+
group.go user goroutine lifecycle helper
31+
```
32+
33+
Before changing an ABI wrapper, inspect the relevant C declarations in
34+
`down/abi_impl/abi.h` and the existing Go implementation. Envoy ABI signatures
35+
are the source of truth for parameter order, return types, and pointer lifetimes.
36+
37+
## Boundary rules
38+
39+
- `down/down.go` and `up/` must never import `C` or use CGO types.
40+
- Only `down/abi_impl/` touches `C`, `unsafe`, and Envoy ABI typedefs.
41+
- Do not blank-import `github.com/dio/transit/down/abi_impl` from library
42+
packages such as `up`. The blank import belongs in the `cmd/main.go` that is
43+
built with `-buildmode=c-shared`.
44+
- Reason: Linux linkers reject ordinary test binaries that contain unresolved
45+
`envoy_dynamic_module_callback_*` references. Those symbols are provided by
46+
the running Envoy process only when the module is loaded as a `.so`; macOS may
47+
allow this accidentally, so Linux CI is the guardrail.
48+
- `down/abi_impl/internal.go` must keep platform linker allowances for package
49+
tests: Darwin uses `-Wl,-undefined,dynamic_lookup`; Linux uses
50+
`-Wl,--unresolved-symbols=ignore-all`. Without the Linux flag,
51+
`go test -race ./...` fails while linking `down/abi_impl.test` even though
52+
the callbacks are never invoked by unit tests.
53+
- Public APIs use transit types such as `HostPtr`, `HostSpec`,
54+
`ClusterLBCompletion`, `ClusterHandle`, `ClusterLBHandle`, `LBHandle`,
55+
`ClusterLBContext`, and `LBContext`.
56+
- `up/cluster.go` and `up/lb.go` should remain thin aliases/re-exports over
57+
`down` to avoid import cycles.
58+
- Registration functions must panic on duplicate names, matching the existing
59+
`RegisterAccessLoggerConfigFactory`, `RegisterCluster`, and
60+
`RegisterLBPolicy` behavior.
61+
62+
## Pointer and memory rules
63+
64+
- Use `manager[T]` for Envoy-to-Go module pointer round trips. Record wrapper
65+
objects on `*_new`/`*_config_new` and remove them on matching destroy/cancel
66+
paths.
67+
- Do not hand arbitrary Go pointers to Envoy outside the existing managed
68+
wrapper pattern.
69+
- Stack-allocate transient handles such as `dymClusterLBHandle`,
70+
`dymClusterLBContext`, `dymLBHandle`, and `dymLBContext` inside callbacks.
71+
They are valid only for the current Envoy callback unless the API explicitly
72+
supports async use.
73+
- Convert Envoy buffers with `envoyBufferToStringUnsafe` when the string must
74+
survive the callback; it clones today. Use `envoyBufferToUnsafeEnvoyBuffer`
75+
only for APIs that document callback-scoped lifetime.
76+
- After passing Go strings or byte slices to C with `stringToModuleBuffer` or
77+
`bytesToModuleBuffer`, call `runtime.KeepAlive` after the C call.
78+
79+
## Cluster Extension lifecycle
80+
81+
Cluster Extension state is split into three managed wrapper levels:
82+
83+
- `clusterConfigWrapper`: per config block, owns `down.ClusterConfigFactory`.
84+
- `clusterWrapper`: per cluster, owns `down.Cluster` and `clusterHandleImpl`.
85+
- `clusterLBWrapper`: per worker LB, owns `down.ClusterLB` and the Envoy LB ptr.
86+
87+
The public API flow is:
88+
89+
1. `up.RegisterCluster(name, down.ClusterFactory)` registers a config parser.
90+
2. `envoy_dynamic_module_on_cluster_config_new` parses config via
91+
`ClusterFactory.Create`.
92+
3. `envoy_dynamic_module_on_cluster_new` creates `ClusterHandle` and calls
93+
`ClusterConfigFactory.NewCluster`.
94+
4. Envoy lifecycle exports call `Cluster.Init`, `ServerInitialized`,
95+
`DrainStarted`, `Shutdown`, and `Close`.
96+
5. `envoy_dynamic_module_on_cluster_lb_new` calls `Cluster.NewClusterLB`.
97+
6. Worker callbacks call `ClusterLB.ChooseHost`, `CancelHostSelection`,
98+
`OnHostMembershipUpdate`, and `Close`.
99+
100+
Implemented Cluster Extension exports:
101+
102+
```
103+
envoy_dynamic_module_on_cluster_config_new
104+
envoy_dynamic_module_on_cluster_config_destroy
105+
envoy_dynamic_module_on_cluster_new
106+
envoy_dynamic_module_on_cluster_init
107+
envoy_dynamic_module_on_cluster_destroy
108+
envoy_dynamic_module_on_cluster_server_initialized
109+
envoy_dynamic_module_on_cluster_drain_started
110+
envoy_dynamic_module_on_cluster_shutdown
111+
envoy_dynamic_module_on_cluster_scheduled
112+
envoy_dynamic_module_on_cluster_lb_new
113+
envoy_dynamic_module_on_cluster_lb_destroy
114+
envoy_dynamic_module_on_cluster_lb_choose_host
115+
envoy_dynamic_module_on_cluster_lb_cancel_host_selection
116+
envoy_dynamic_module_on_cluster_lb_on_host_membership_update
117+
envoy_dynamic_module_on_cluster_http_callout_done
118+
```
119+
120+
`envoy_dynamic_module_on_cluster_http_callout_done` is currently a required
121+
stub; transit does not expose cluster HTTP callouts.
122+
123+
## Cluster main-thread scheduling
124+
125+
`ClusterHandle` methods that mutate cluster state must run on Envoy's main
126+
thread: `AddHosts`, `RemoveHosts`, `UpdateHostHealth`, `PreInitComplete`, and
127+
calls that depend on Envoy cluster main-thread ownership.
128+
129+
From background goroutines or worker callbacks, use `ClusterHandle.Schedule(fn)`.
130+
It stores `fn` in `clusterHandleImpl.pending`, commits through Envoy's cluster
131+
scheduler, then runs `fn` from `envoy_dynamic_module_on_cluster_scheduled`.
132+
133+
Use `up.Group` for user-owned background goroutines. Start it from the user's
134+
cluster lifecycle code and stop it from `Close`/`Shutdown`; do not add generic
135+
goroutine management to `abi_impl`.
136+
137+
## Async ClusterLB host selection
138+
139+
`ClusterLB.ChooseHost` returns:
140+
141+
- `(host, nil)` for synchronous success.
142+
- `(nil, nil)` for synchronous no-host/failure.
143+
- `(nil, completion)` for async selection.
144+
145+
For async paths:
146+
147+
- Create the completion with `ctx.NewCompletion()` so it captures the Envoy LB
148+
and request context pointers needed for completion.
149+
- `abi_impl` records an `asyncHandleWrapper` in `clusterAsyncManager`, attaches
150+
a finish hook with `completion.SetFinishFn`, and returns the managed async
151+
handle to Envoy.
152+
- `completion.Complete(host, detail)` must call Envoy exactly once.
153+
- `envoy_dynamic_module_on_cluster_lb_cancel_host_selection` calls
154+
`completion.Cancel()` first; only if that wins should it call user
155+
`ClusterLB.CancelHostSelection(completion)`.
156+
- Completion cleanup must happen on both Complete and Cancel paths.
157+
158+
Never call Envoy async completion after cancellation or after the finish hook has
159+
removed the async wrapper.
160+
161+
## LB Policy lifecycle
162+
163+
LB Policy is smaller than Cluster Extension:
164+
165+
- `lbConfigWrapper`: per policy config, owns `down.LBPolicyConfigFactory`.
166+
- `lbWrapper`: per worker LB, owns `down.LBPolicy` and the Envoy LB ptr.
167+
168+
The public API flow is:
169+
170+
1. `up.RegisterLBPolicy(name, down.LBPolicyFactory)` registers a config parser.
171+
2. `envoy_dynamic_module_on_lb_config_new` parses config via
172+
`LBPolicyFactory.Create`.
173+
3. `envoy_dynamic_module_on_lb_new` calls `LBPolicyConfigFactory.NewLBPolicy`.
174+
4. Worker callbacks call `LBPolicy.ChooseHost`, `OnHostMembershipUpdate`, and
175+
`Close`.
176+
177+
Implemented LB Policy exports:
178+
179+
```
180+
envoy_dynamic_module_on_lb_config_new
181+
envoy_dynamic_module_on_lb_config_destroy
182+
envoy_dynamic_module_on_lb_new
183+
envoy_dynamic_module_on_lb_destroy
184+
envoy_dynamic_module_on_lb_choose_host
185+
envoy_dynamic_module_on_lb_on_host_membership_update
186+
```
187+
188+
LB Policy `ChooseHost` writes priority/index and returns `true`; returning
189+
`false` means no host. It does not support async host selection. `LBContext`
190+
does not expose filter state or downstream SNI; use Cluster Extension for that.
191+
192+
## Handle callback coverage
193+
194+
Cluster and LB handles intentionally share many concepts: priority count, total
195+
hosts, healthy/degraded hosts, address lookup, weights, health, host stats,
196+
metadata, locality, per-host opaque data, and membership update addresses.
197+
198+
When adding a handle method:
199+
200+
1. Add the public method to `down/down.go`.
201+
2. Implement it for both `dymClusterLBHandle` and `dymLBHandle` if Envoy exposes
202+
equivalent callbacks for both extension types.
203+
3. Add the alias/re-export in `up/cluster.go` or `up/lb.go` only if users need a
204+
new named type or constant.
205+
4. Preserve callback-scoped lifetime in method comments when returning Envoy
206+
data directly.
207+
208+
When adding a context method, check whether both ABI contexts support it.
209+
Cluster LB has filter state and downstream SNI; LB Policy currently does not.
210+
211+
## Validation
212+
213+
Run focused tests after wrapper changes:
214+
215+
```
216+
go test ./down/abi_impl ./down ./up
217+
```
218+
219+
For broader API or lifecycle changes, run the repository's normal test target:
220+
221+
```
222+
go test -race ./...
223+
```
224+
225+
Use `transit-unit-testing` when changing unit tests or debugging ordinary
226+
`go test` linker failures around `down/abi_impl`.
227+
228+
If `abi.h` changed via `make update-sdk`, inspect the relevant declarations with
229+
`rg` and re-check every affected `//export` function and callback invocation.
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
---
2+
name: transit-e2e-authoring
3+
description: Create or extend transit end-to-end tests. Use when adding e2e filters, Envoy test config, sinks, assertions, or example e2e suites for transit dynamic modules.
4+
---
5+
6+
# Transit E2E Authoring
7+
8+
Use this skill when adding new e2e coverage for transit or its examples.
9+
10+
## Choose the right suite
11+
12+
- Use root `e2e/` for core transit API behavior, ABI wrapper coverage, access
13+
logger behavior, body handling, metadata, telemetry, upstream filters, and LB
14+
Policy behavior.
15+
- Use `examples/<name>/e2e/` when validating a specific example's user-facing
16+
behavior.
17+
- Add a new sink under `e2e/sinks/` only when existing HTTP JSON, OTLP, or ALS
18+
sinks cannot observe the behavior.
19+
20+
## Root e2e structure
21+
22+
```
23+
e2e/
24+
cmd/main.go imports e2e/filters for shared-library build
25+
filters/*.go test dynamic modules registered in init
26+
testdata/envoy.yaml.tmpl Envoy bootstrap template
27+
main_test.go builds libe2e.so, starts Envoy and sinks
28+
*_test.go feature assertions
29+
sinks/* in-process assertion sinks
30+
```
31+
32+
To add a root e2e case:
33+
34+
1. Add or update a test filter in `e2e/filters`. Register it with the same public
35+
API a user would call (`up.Register`, `RegisterWithBody`,
36+
`RegisterAccessLogger`, `RegisterLBPolicy`, etc.).
37+
2. Ensure `e2e/cmd/main.go` blank-imports both
38+
`github.com/dio/transit/down/abi_impl` and
39+
`github.com/dio/transit/e2e/filters`. The `abi_impl` blank import belongs in
40+
the shared-library entrypoint, not in `up` or another library package.
41+
3. Add listener, filter, cluster, access logger, or sink config to
42+
`e2e/testdata/envoy.yaml.tmpl`.
43+
4. Add a port field and `freePort()` allocation in `e2e/main_test.go` when the
44+
test needs a new listener, admin endpoint, or upstream.
45+
5. Add a focused `*_test.go` file or extend the relevant suite.
46+
6. Run `make e2e` or a direct package command with `ENVOY_BIN`.
47+
48+
Keep test filter names stable and obvious: `e2e-<feature>` for filters and
49+
`<feature>-e2e` for listener/stat prefixes.
50+
51+
## Assertions
52+
53+
- Prefer black-box HTTP requests through Envoy over direct calls into filter
54+
code.
55+
- Use existing helpers such as `mustDo`, `readBody`, `waitReady`, and sink
56+
`Wait*` methods when available.
57+
- For asynchronous telemetry or access-log assertions, use bounded waits with
58+
predicates instead of sleeps.
59+
- Reset shared sinks between tests if data from earlier requests can satisfy a
60+
later predicate.
61+
- Assert both the externally visible behavior and the specific transit feature
62+
under test when possible.
63+
64+
## Example e2e structure
65+
66+
Example e2e suites usually live at:
67+
68+
```
69+
examples/<name>/e2e/e2e_test.go
70+
examples/<name>/e2e/testdata/envoy.yaml.tmpl
71+
```
72+
73+
The example `TestMain` normally:
74+
75+
- locates `examples/` with `runtime.Caller`,
76+
- checks `ENVOY_BIN` or `../.bin/envoy`,
77+
- builds `lib<name>.so` with `go build -trimpath -buildmode=c-shared`; the
78+
example `cmd/main.go` must blank-import `github.com/dio/transit/down/abi_impl`
79+
so Envoy ABI exports are linked only into the `.so`,
80+
- starts any in-process upstream/test server,
81+
- renders a temp Envoy config,
82+
- starts Envoy with `GODEBUG=cgocheck=0` and
83+
`ENVOY_DYNAMIC_MODULES_SEARCH_PATH=<example dir>`,
84+
- waits for admin `/ready`,
85+
- runs tests and tears Envoy down.
86+
87+
Reuse the nearest existing example e2e harness (`hello`, `lb-policy`,
88+
`sse-tap`, or `request-ui`) rather than writing a new harness from memory.
89+
90+
If an e2e example depends on embedded static assets, keep a minimal tracked
91+
fixture under the embedded path. CI lint/typecheck runs from a clean checkout
92+
and will fail on `//go:embed` patterns that only exist after a local asset build.
93+
94+
## Validation commands
95+
96+
Root e2e:
97+
98+
```
99+
make e2e
100+
```
101+
102+
Example e2e:
103+
104+
```
105+
make e2e-hello
106+
make e2e-sse-tap
107+
make e2e-request-ui
108+
make e2e-lb-policy
109+
```
110+
111+
Fast rerun after a successful build:
112+
113+
```
114+
TRANSIT_SKIP_BUILD=1 make e2e
115+
```
116+
117+
Do not rely on `TRANSIT_SKIP_BUILD=1` for final verification after code changes.

0 commit comments

Comments
 (0)