mirror of
https://github.com/netbirdio/netbird.git
synced 2026-07-16 19:49:56 +00:00
[agent-network] Polish module docs: remove internal review scaffolding, fix links, verify diagrams
Strip PR-review framing, commit references, absolute paths, and stale internal references from the agent-network module docs; fix broken relative links; verify all diagrams against the current architecture. Remove the internal AI-reviewer prompt file.
This commit is contained in:
mlsmaycon
@@ -1,26 +1,23 @@
|
||||
# agent-networks PR — overview
|
||||
# Agent Networks — overview
|
||||
|
||||
Single-entry point for reviewers. Scope, commit roll-up, ownership
|
||||
matrix, cross-cutting risk hot-list, and links into every per-module
|
||||
guide.
|
||||
Single-entry point. Feature scope, the module map, and the cross-cutting
|
||||
topics worth keeping in mind, with links into every per-module guide.
|
||||
|
||||
## TL;DR
|
||||
|
||||
This PR pair introduces an **LLM-aware reverse-proxy middleware system**
|
||||
Agent Networks introduces an **LLM-aware reverse-proxy middleware system**
|
||||
plus **account-level controls** (budget rules, log collection toggles,
|
||||
PII redaction) for NetBird agent-networks. The management server
|
||||
synthesises a per-peer middleware chain that the proxy executes on
|
||||
every LLM request; the chain enforces quotas, injects identity, redacts
|
||||
PII, parses tokens/cost, and emits access-log entries. The dashboard
|
||||
exposes the surface as a single **AI Observability** page with four
|
||||
tabs.
|
||||
PII redaction). The management server synthesises a per-peer middleware
|
||||
chain that the proxy executes on every LLM request; the chain enforces
|
||||
quotas, injects identity, redacts PII, parses tokens/cost, and emits
|
||||
access-log entries. The dashboard exposes the surface as a single **AI
|
||||
Observability** page with four tabs.
|
||||
|
||||
- **Backend branch:** `feature/agent-networks-backend` on
|
||||
`/Users/maycon/projects/netbird` — 28 commits vs merge-base
|
||||
`14af17955`, **~28,000 net LOC added** across 150 files.
|
||||
- **Dashboard branch:** `feature/agent-networks` on
|
||||
`/Users/maycon/projects/dashboard` — ~70 commits vs `main`,
|
||||
**~10,000 net LOC added** across 82 files.
|
||||
- **Backend** lives in this repo, primarily under
|
||||
`management/server/agentnetwork`, `proxy/internal/middleware`, and
|
||||
`proxy/internal/llm`, with wire contracts in `shared/management`.
|
||||
- **Dashboard** lives in the dashboard repo under
|
||||
`src/modules/agent-network/` and `src/app/(dashboard)/agent-network/`.
|
||||
|
||||
## Reading order
|
||||
|
||||
@@ -33,95 +30,44 @@ tabs.
|
||||
| 5 | [modules/31-proxy-middleware-builtin.md](modules/31-proxy-middleware-builtin.md) | The 8 LLM middlewares that ride on the framework. |
|
||||
| 6 | Everything else in any order. | |
|
||||
|
||||
## Ownership matrix
|
||||
## Module map
|
||||
|
||||
11 review-able modules. Each is described in detail in its own file
|
||||
under [`modules/`](modules/).
|
||||
11 modules. Each is described in detail in its own file under
|
||||
[`modules/`](modules/).
|
||||
|
||||
| # | Module | Suggested reviewer | Expertise required | Est. time | Risk | BC impact |
|
||||
|---|--------|--------------------|--------------------|-----------|------|-----------|
|
||||
| 10 | [shared/api](modules/10-shared-api.md) — proto + OpenAPI | API/contracts maintainer | protobuf + oapi-codegen + wire-format reasoning | 30 min | Low | Additive only |
|
||||
| 20 | [management/store](modules/20-management-store.md) — SQL persistence | Storage maintainer | gorm + sqlite/postgres + migrations | 45 min | Medium | Auto-migrate (additive) |
|
||||
| 21 | [management/agentnetwork](modules/21-management-agentnetwork.md) — domain layer + synthesizer | Management server maintainer | network-map controller + policy engine + Go concurrency | **2–3 h** | **High** | Additive |
|
||||
| 22 | [management/handlers + wiring](modules/22-management-handlers-wiring.md) — HTTP API + gRPC delivery | Management infra maintainer | REST handlers + RBAC + gRPC streaming + controller wiring | 90 min | Medium | Additive |
|
||||
| 30 | [proxy/middleware-framework](modules/30-proxy-middleware-framework.md) — generic plugin system | Proxy maintainer | net/http composition + request lifecycle + Go context | 60 min | High | Additive |
|
||||
| 31 | [proxy/middleware-builtin](modules/31-proxy-middleware-builtin.md) — 8 LLM middlewares | Proxy + LLM owner | OpenAI/Anthropic semantics + the framework above | **2 h** | High | Additive |
|
||||
| 32 | [proxy/llm-parsers](modules/32-proxy-llm-parsers.md) — SDK adapters + pricing | LLM/proxy owner | OpenAI Responses API + Anthropic Messages API + SSE framing | 75 min | Medium | Additive |
|
||||
| 33 | [proxy/runtime](modules/33-proxy-runtime.md) — translate + serve + access-log | Proxy maintainer | existing reverseproxy.go lifecycle + access-log emission + synth ingestion | 90 min | High | Additive (touches hot path) |
|
||||
| 40 | [dashboard](modules/40-dashboard.md) — UI for everything above | Dashboard maintainer | Next.js app router + existing Tabs/Modal/Table patterns + Providers/Permissions/Groups context | **2 h** | Medium | Sidebar reshape |
|
||||
| 50 | [path-routed-providers](modules/50-path-routed-providers.md) — Vertex AI + Bedrock | Proxy + LLM owner | URL-path routing + GCP OAuth + AWS Bedrock runtime + catalog↔pricing | 45 min | Medium | Additive (new catalog entries) |
|
||||
| — | E2E scripts under `scripts/e2e/agent-network-policy/` | Whoever runs the e2e suite | bash + tilt | 30 min | Low | New only |
|
||||
| # | Module | Risk | BC impact |
|
||||
|---|--------|------|-----------|
|
||||
| 10 | [shared/api](modules/10-shared-api.md) — proto + OpenAPI | Low | Additive only |
|
||||
| 20 | [management/store](modules/20-management-store.md) — SQL persistence | Medium | Auto-migrate (additive) |
|
||||
| 21 | [management/agentnetwork](modules/21-management-agentnetwork.md) — domain layer + synthesizer | **High** | Additive |
|
||||
| 22 | [management/handlers + wiring](modules/22-management-handlers-wiring.md) — HTTP API + gRPC delivery | Medium | Additive |
|
||||
| 30 | [proxy/middleware-framework](modules/30-proxy-middleware-framework.md) — generic plugin system | High | Additive |
|
||||
| 31 | [proxy/middleware-builtin](modules/31-proxy-middleware-builtin.md) — 8 LLM middlewares | High | Additive |
|
||||
| 32 | [proxy/llm-parsers](modules/32-proxy-llm-parsers.md) — SDK adapters + pricing | Medium | Additive |
|
||||
| 33 | [proxy/runtime](modules/33-proxy-runtime.md) — translate + serve + access-log | High | Additive (touches hot path) |
|
||||
| 40 | [dashboard](modules/40-dashboard.md) — UI for everything above | Medium | Sidebar reshape |
|
||||
| 50 | [path-routed-providers](modules/50-path-routed-providers.md) — Vertex AI + Bedrock | Medium | Additive (new catalog entries) |
|
||||
|
||||
**Total wall-clock review estimate:** ~13–15 hours if reviewers
|
||||
serialize. With per-module fan-out (one reviewer per row) the critical
|
||||
path is ~3 hours (the management/agentnetwork module).
|
||||
The largest and highest-risk module is `management/agentnetwork`: it is
|
||||
the single writer of the middleware chain the proxy executes.
|
||||
|
||||
## Commit roll-up
|
||||
## Cross-cutting topics
|
||||
|
||||
### Backend (28 commits, oldest → newest)
|
||||
|
||||
| SHA | Module(s) | Subject |
|
||||
|-----|-----------|---------|
|
||||
| `06ff17b38` | shared/api, agentnetwork/types, proxy/llm | AN-0: additive proto + OpenAPI schemas + base types |
|
||||
| `f810a4e35` | management/store | AN-1: store layer for providers/policies/guardrails/settings |
|
||||
| `77b407632` | management/agentnetwork | AN-2: agentnetwork module (manager, synthesizer, catalog, policyselect) |
|
||||
| `09e8059b6` | management/handlers+wiring | AN-2b: wire synth services into the network map |
|
||||
| `9ecb6449d` | management/handlers+wiring | AN-3: HTTP API handlers + route registration |
|
||||
| `00bf0d328` | proxy/middleware-framework | AN-4: proxy middleware framework |
|
||||
| `3ed29d855` | proxy/runtime | AN-4b: middleware plumbing in the proxy request path |
|
||||
| `e64ea4b02` | proxy/middleware-builtin | AN-5: built-in middlewares + registry registration |
|
||||
| `0f9f56a58` | proxy/runtime | AN-5b: activate the middleware system in the proxy server |
|
||||
| `9a1547143` | shared/api, proxy/runtime, accesslog | AN-6: access-log agent_network flag end-to-end |
|
||||
| `8cb9c187d` | shared/api, agentnetwork, runtime | AN-7: enforcement + synth-service delivery to the proxy |
|
||||
| `9ebe219fd` | proxy/runtime | test: lock the auth→middleware group-propagation wiring |
|
||||
| `665575932` | agentnetwork tests | test: real-store integration coverage (no MockStore) |
|
||||
| `263dabd73` | proxy/runtime | fix: preserve ProxyMapping.Private on per-proxy live updates |
|
||||
| `5adee2cb4` | agentnetwork tests | Add no-mock agent-network provider-CRUD fan-out test |
|
||||
| `9ae476ea7` | agentnetwork tests | Add no-mock baseline guards for enforcement and guardrail synthesis |
|
||||
| `a436b5fb3` | agentnetwork, store, shared/api | GC-0: account budget rule type + collection toggles + store CRUD |
|
||||
| `5b408b0ef` | agentnetwork | GC-1: budget-rule manager CRUD + settings update |
|
||||
| `b22d5a181` | agentnetwork, builtin | GC-2: enforce account budget rules as a min-wins ceiling |
|
||||
| `945f17f1a` | agentnetwork, store, builtin | GC-3: account-level prompt-collection master switch |
|
||||
| `23bdf6871` | handlers, agentnetwork, shared/api | GC-4: HTTP API for budget rules + settings update |
|
||||
| `468875cb4` | management+proxy | Wire EnableLogCollection to suppress access-log for opted-out requests |
|
||||
| `7072f8125` | agentnetwork, builtin | Account toggle is sole control for prompt capture + broader PII redaction |
|
||||
| `b1e66bca2` | builtin, agentnetwork | Broaden phone redactor + fixture-driven Go test + smoke --groups |
|
||||
| `b438a7194` | agentnetwork, proxy/runtime | Fix budget enforcement + extend PII redaction to all metadata channels |
|
||||
| `19e03d688` | agentnetwork tests | Self-contained Go tests for the redact-pii wiring across parsers |
|
||||
| `2a0d4991b` | proxy/runtime | Self-contained full-chain integration test for agent-network requests |
|
||||
| `4836d5a19` | agentnetwork, builtin | Gate prompt + completion capture on EnablePromptCollection |
|
||||
|
||||
### Dashboard (recent → oldest, abbreviated)
|
||||
|
||||
- `3fbe44e` Merge Global Controls + Access Log into AI Observability (today)
|
||||
- `1988e92` agent-network: Global Controls page with account toggles + budget rules
|
||||
- `2157fec` Consumption page: charts + filters + access-log style
|
||||
- `62a85b5` add policy Limits tab; slim Guardrails to allowlist + capture
|
||||
- `2b7f746` Wire Agent Network guardrails to backend
|
||||
- `5e10a2b` Wire Agent Network policies to backend
|
||||
- `4d3ac8d` Wire Agent Network providers to backend API
|
||||
- …plus ~50 other UI iterations
|
||||
- Full list: `git -C /Users/maycon/projects/dashboard log --oneline main..HEAD`
|
||||
|
||||
## Risk hot-list (cross-cutting)
|
||||
|
||||
Pulled from the per-module guides; these are the items most likely to
|
||||
bite production. Each is fully documented in the linked module guide.
|
||||
These are the items most likely to bite production. Each is fully
|
||||
documented in the linked module guide.
|
||||
|
||||
1. **Capture-pointer semantics** (`*bool` for `capture_prompt` and
|
||||
`capture_completion`): nil = legacy emit, false = suppress, true =
|
||||
emit. Fixed at `4836d5a19`. Reviewers should verify nil-vs-false is
|
||||
handled at every json hop. See
|
||||
emit. nil-vs-false must be handled at every JSON hop. See
|
||||
[21-management-agentnetwork.md](modules/21-management-agentnetwork.md)
|
||||
and [31-proxy-middleware-builtin.md](modules/31-proxy-middleware-builtin.md).
|
||||
2. **ProxyMapping.Private preservation** on per-proxy live updates.
|
||||
Fix at `263dabd73`. Failure mode: `auth` skips
|
||||
`ValidateTunnelPeer` → `CapturedData.UserGroups` empty → `llm_router`
|
||||
denies. See [33-proxy-runtime.md](modules/33-proxy-runtime.md).
|
||||
2. **`ProxyMapping.Private` preservation** on per-proxy live updates.
|
||||
Failure mode: `auth` skips `ValidateTunnelPeer` →
|
||||
`CapturedData.UserGroups` empty → `llm_router` denies. See
|
||||
[33-proxy-runtime.md](modules/33-proxy-runtime.md).
|
||||
3. **respInput carrying `UserEmail`/`UserGroups`/`UserGroupNames` onto
|
||||
the response leg** at `reverseproxy.go:196-223` (fix at `b438a7194`).
|
||||
Load-bearing wire that lets `llm_limit_record` ship non-empty
|
||||
`group_ids` on `RecordLLMUsage`. See
|
||||
the response leg** in `reverseproxy.go`. Load-bearing wire that lets
|
||||
`llm_limit_record` ship non-empty `group_ids` on `RecordLLMUsage`. See
|
||||
[33-proxy-runtime.md](modules/33-proxy-runtime.md).
|
||||
4. **Min-wins all-must-pass budget rule semantics**. Every matching
|
||||
rule's remaining quota must be > 0 for the request to proceed; one
|
||||
@@ -133,66 +79,31 @@ bite production. Each is fully documented in the linked module guide.
|
||||
budget, `LimitReader(r.Body, limit+1)` for truncation detection with
|
||||
`replayReadCloser` fallback so upstream still sees the full body.
|
||||
`cloneInputFor` deep-copies the body up to 16 times per chain — a
|
||||
perf hot-spot worth confirming benchmarks for. See
|
||||
perf hot-spot. See
|
||||
[30-proxy-middleware-framework.md](modules/30-proxy-middleware-framework.md).
|
||||
6. **UpstreamRewrite.AuthHeader bypasses the header denylist**
|
||||
deliberately. Confirm the runtime consumer only unpacks it via the
|
||||
deliberately. The runtime consumer only unpacks it via the
|
||||
trusted upstream-build path. See
|
||||
[30-proxy-middleware-framework.md](modules/30-proxy-middleware-framework.md).
|
||||
7. **Codegen reproducibility gap**:
|
||||
`shared/management/http/api/generate.sh:14` pulls
|
||||
`oapi-codegen@latest` while the AN-0 commit message claims v2.7.0 is
|
||||
pinned. A future regen could produce a different `types.gen.go`. See
|
||||
7. **`disable_access_log` default-false semantics**: the synth target
|
||||
sets it true, all other targets leave it false. See
|
||||
[10-shared-api.md](modules/10-shared-api.md).
|
||||
8. **`disable_access_log` default-false semantics**: synth target sets
|
||||
it true, all other targets must leave it false. Worth a
|
||||
synthesizer-side audit. See
|
||||
[10-shared-api.md](modules/10-shared-api.md).
|
||||
9. **String-typed `decision` / `deny_code`** on
|
||||
8. **String-typed `decision` / `deny_code`** on
|
||||
`CheckLLMPolicyLimitsResponse` — would benefit from enum pinning
|
||||
before external consumers integrate. See
|
||||
[10-shared-api.md](modules/10-shared-api.md).
|
||||
10. **agent_network_chain_realstack_test drift risk**: it inlines the
|
||||
proto→Spec mapping instead of calling
|
||||
`proxy/middleware_translate.go`. Test will pass even if translate
|
||||
regresses. See [33-proxy-runtime.md](modules/33-proxy-runtime.md).
|
||||
11. **MOCK_GROUPS still in production paths** via
|
||||
`AgentPoliciesTable.tsx:45,76` as a name-lookup fallback. Other
|
||||
`MOCK_*` constants are unreferenced. See
|
||||
[40-dashboard.md](modules/40-dashboard.md).
|
||||
12. **`updateProvider` / `updatePolicy` / `updateBudgetRule` use `??`
|
||||
on `enabled`** — toggle paths are safe but an explicit
|
||||
`enabled:false` from a form would be silently dropped. See
|
||||
[40-dashboard.md](modules/40-dashboard.md).
|
||||
13. **Dashboard lint gate is broken** (`next lint` was removed in Next
|
||||
16). Pre-existing, but this branch effectively ships without a
|
||||
lint check. See [40-dashboard.md](modules/40-dashboard.md).
|
||||
14. **Zero Cypress / component test coverage for new agent-network
|
||||
UI.** See [40-dashboard.md](modules/40-dashboard.md).
|
||||
15. **Bodytap truncation-replay test missing**; **Dispatcher
|
||||
timeout/panic tests missing**; **concurrent Budget exhaustion
|
||||
test missing**; **InvalidateMiddleware + LiveServiceCheck race
|
||||
test missing**. See
|
||||
[30-proxy-middleware-framework.md](modules/30-proxy-middleware-framework.md).
|
||||
|
||||
## Explicit non-goals
|
||||
|
||||
- **Reaper / GC pass over stale synth services** — designed but scope-cut
|
||||
per AN-0 commit message; not migrated.
|
||||
- **Reaper / GC pass over stale synth services** — designed but cut from
|
||||
scope.
|
||||
- **URL-sync for tab state on AI Observability** — read path is wired
|
||||
(`?tab=`) but write path isn't. Future work.
|
||||
- **Lint-gate restoration on the dashboard** — pre-existing,
|
||||
out-of-scope.
|
||||
- **CI golden-file regen-and-diff for `types.gen.go` /
|
||||
`proxy_service.pb.go`** — would catch codegen drift; not in this PR.
|
||||
`proxy_service.pb.go`** — would catch codegen drift; not yet in place.
|
||||
|
||||
## Where to read the code
|
||||
|
||||
- Backend: `git -C /Users/maycon/projects/netbird diff 14af17955..HEAD`
|
||||
- Dashboard: `git -C /Users/maycon/projects/dashboard diff main..HEAD`
|
||||
- Per-module file scopes are listed in each module guide.
|
||||
|
||||
## Where this doc lives
|
||||
|
||||
`~/projects/claude-contexts/netbird/agent-networks-pr/`. Not committed
|
||||
to either repo. Move pieces into `docs/` if/when ready.
|
||||
Per-module file scopes are listed in each module guide. Behaviour is
|
||||
covered by Go tests co-located with each package (and an end-to-end
|
||||
chain integration test under `proxy/internal/proxy`).
|
||||
|
||||
@@ -141,7 +141,7 @@ sequenceDiagram
|
||||
- Access-log emission is gated on `settings.EnableLogCollection`. With
|
||||
it OFF, neither the deny nor the allow leg writes an entry — the
|
||||
chain still runs (budget rules are still enforced) but no audit trail
|
||||
is kept. See `468875cb4` and
|
||||
is kept. See
|
||||
[`modules/33-proxy-runtime.md`](modules/33-proxy-runtime.md).
|
||||
|
||||
---
|
||||
@@ -194,8 +194,7 @@ flowchart LR
|
||||
to (group set, user set) with a (window, ceiling). At check time,
|
||||
every rule that matches the caller is evaluated; if ANY rule has
|
||||
zero remaining quota the request is denied. This is the most
|
||||
surprising semantic for operators — see GC-2 (`b22d5a181`) and the
|
||||
invariants section of
|
||||
surprising semantic for operators — see the invariants section of
|
||||
[`modules/21-management-agentnetwork.md`](modules/21-management-agentnetwork.md).
|
||||
- The proxy never makes its own budget decisions. It always asks
|
||||
management via `CheckLLMPolicyLimits` and reports back via
|
||||
@@ -203,8 +202,7 @@ flowchart LR
|
||||
and avoids per-proxy drift.
|
||||
- `RecordLLMUsage` must carry `group_ids` and `user_id` so the
|
||||
decrement hits the right rule(s). The wire that carries those
|
||||
fields onto the response leg is `respInput` in `reverseproxy.go`,
|
||||
fixed at `b438a7194`. See
|
||||
fields onto the response leg is `respInput` in `reverseproxy.go`. See
|
||||
[`modules/33-proxy-runtime.md`](modules/33-proxy-runtime.md).
|
||||
- The dashboard's Budget Dashboard tab polls
|
||||
`/api/agent-network/consumption` — not gRPC, not WebSocket. Poll
|
||||
@@ -216,5 +214,4 @@ flowchart LR
|
||||
## Cross-references
|
||||
|
||||
- Per-module guides: [`modules/`](modules/)
|
||||
- Overview + ownership matrix: [`00-overview.md`](00-overview.md)
|
||||
- AI reviewer prompt: [`90-agent-review-prompt.md`](90-agent-review-prompt.md)
|
||||
- Overview + module map: [`00-overview.md`](00-overview.md)
|
||||
|
||||
@@ -1,246 +0,0 @@
|
||||
# Prompt for an AI code reviewer
|
||||
|
||||
Self-contained prompt to feed to a second AI agent (Claude, GPT, etc.)
|
||||
so it can perform structured code review of the agent-networks PR in
|
||||
parallel with the human reviewers. Copy the **Prompt** block below into
|
||||
the agent's first message. The agent should produce a single markdown
|
||||
report at a path you specify.
|
||||
|
||||
The prompt assumes the agent has filesystem read access to the netbird
|
||||
and dashboard repos and can run `git`, but does NOT have permission to
|
||||
modify or commit anything.
|
||||
|
||||
---
|
||||
|
||||
## Prompt
|
||||
|
||||
```
|
||||
You are reviewing a large PR pair for security, correctness, concurrency,
|
||||
backward-compatibility, performance, and observability. You are read-only
|
||||
on both repos: do NOT modify any file, do NOT commit anything, do NOT
|
||||
stage anything, do NOT run e2e scripts. Only read code and emit a single
|
||||
markdown report at:
|
||||
|
||||
/Users/maycon/projects/claude-contexts/netbird/agent-networks-pr/AGENT_REVIEW_REPORT.md
|
||||
|
||||
REPOS
|
||||
Backend: /Users/maycon/projects/netbird (branch feature/agent-networks-backend, merge-base 14af17955)
|
||||
Dashboard: /Users/maycon/projects/dashboard (branch feature/agent-networks, merge-base main)
|
||||
|
||||
PR CONTEXT
|
||||
Backend: 28 commits, ~28k LOC added across 150 files.
|
||||
Dashboard: ~70 commits, ~10k LOC added across 82 files.
|
||||
The PR introduces an LLM-aware reverse-proxy middleware system for
|
||||
NetBird agent-networks plus account-level controls (budget rules,
|
||||
PII redaction, log + prompt capture toggles). The management server
|
||||
synthesises a per-peer middleware chain that the proxy executes for
|
||||
every LLM request; the chain enforces quotas, injects identity,
|
||||
redacts PII, parses tokens / cost, and emits access-log entries.
|
||||
|
||||
REQUIRED INPUT — read these BEFORE reading any code
|
||||
|
||||
/Users/maycon/projects/claude-contexts/netbird/agent-networks-pr/README.md
|
||||
/Users/maycon/projects/claude-contexts/netbird/agent-networks-pr/00-overview.md
|
||||
/Users/maycon/projects/claude-contexts/netbird/agent-networks-pr/01-end-to-end-flows.md
|
||||
|
||||
AND the per-module guide for every module you intend to review:
|
||||
modules/10-shared-api.md
|
||||
modules/20-management-store.md
|
||||
modules/21-management-agentnetwork.md
|
||||
modules/22-management-handlers-wiring.md
|
||||
modules/30-proxy-middleware-framework.md
|
||||
modules/31-proxy-middleware-builtin.md
|
||||
modules/32-proxy-llm-parsers.md
|
||||
modules/33-proxy-runtime.md
|
||||
modules/40-dashboard.md
|
||||
|
||||
Each guide names its reviewer profile, lists commits in scope, shows
|
||||
the file map, names invariants, and enumerates "Things to scrutinize".
|
||||
Use those lists to focus your reading — do not re-derive what the
|
||||
guides already documented.
|
||||
|
||||
YOUR JOB
|
||||
For each module, validate that the invariants in the guide actually
|
||||
hold in the code, and find anything the guide missed. The guide is
|
||||
your CHECKLIST, not the truth — it is itself a draft. Flag any place
|
||||
where the guide is wrong, stale, or misleading.
|
||||
|
||||
FOCUS AREAS (in priority order)
|
||||
1. Security
|
||||
- Privilege boundaries: account isolation in store queries; group
|
||||
resolution can never cross accounts.
|
||||
- PII handling: every metadata channel out of the proxy must
|
||||
respect redact_pii when enabled. Confirm at every JSON-marshal
|
||||
site, every access-log write, every gRPC outbound message.
|
||||
- Capture-pointer semantics (*bool for capture_prompt /
|
||||
capture_completion): nil = legacy emit, false = suppress,
|
||||
true = emit. Verify every json hop preserves nil vs false.
|
||||
- api_key fields on AgentNetworkProviderRequest must be
|
||||
write-only — never echoed on responses.
|
||||
- UpstreamRewrite.AuthHeader deliberately bypasses the header
|
||||
denylist. Confirm consumers only unpack it via the trusted
|
||||
upstream-build path; flag if any code path lets a synth-untrusted
|
||||
caller stage a fake AuthHeader.
|
||||
- Permission keys on every HTTP handler. Confirm permission?.services?.read
|
||||
gates every read; permission?.services?.write gates writes.
|
||||
|
||||
2. Correctness
|
||||
- Min-wins all-must-pass budget rule semantics in
|
||||
management/agentnetwork: every matching rule's remaining quota
|
||||
must be > 0 for the request to proceed. Verify the
|
||||
check + record pair stays consistent (no orphan decrements,
|
||||
no double-count on retries).
|
||||
- llm_limit_check ↔ llm_limit_record pairing — a successful check
|
||||
must always result in a corresponding record (or the rate-limit
|
||||
counters skew). Look for early-returns / panics that could
|
||||
break the pair.
|
||||
- ProxyMapping.Private preservation on per-proxy live updates
|
||||
(263dabd73). Failure mode: auth skips ValidateTunnelPeer →
|
||||
CapturedData.UserGroups empty → llm_router denies. Verify the
|
||||
fix covers every gRPC translation path.
|
||||
- respInput carrying UserEmail/UserGroups/UserGroupNames onto
|
||||
the response leg (reverseproxy.go:196-223, b438a7194). If this
|
||||
wire breaks, llm_limit_record ships empty group_ids and
|
||||
budget decrements miss.
|
||||
- agent_network_chain_realstack_test.go inlines proto→Spec
|
||||
mapping instead of calling middleware_translate.go — confirm
|
||||
this drift risk, suggest fix.
|
||||
|
||||
3. Concurrency
|
||||
- Chain replacement under in-flight requests in proxy/runtime.
|
||||
Confirm no goroutine sees a half-replaced chain.
|
||||
- Body-tap memory bounds: 1 MiB per-direction cap, 256 MiB shared
|
||||
Budget semaphore. Verify the Budget release path runs on every
|
||||
exit (including panic + timeout).
|
||||
- cloneInputFor deep-copies the body up to 16 times per chain.
|
||||
Note allocation cost; flag if benchmarks are missing.
|
||||
- InvalidateMiddleware + LiveServiceCheck race — flag if the
|
||||
guide says no test exists.
|
||||
|
||||
4. Backward compatibility
|
||||
- All proto field numbers in proxy_service.proto are in unused
|
||||
slots; OpenAPI additions are appended; no existing schemas
|
||||
lose required-ness. Confirm by diffing 14af17955..HEAD on
|
||||
shared/management/proto/proxy_service.proto and
|
||||
shared/management/http/api/openapi.yml.
|
||||
- Pointer fields for capture flags preserve nil-default emission
|
||||
so existing proxies that don't know about the field keep
|
||||
working.
|
||||
- Non-agent-network services on the proxy must still work.
|
||||
Verify ServiceMapping doesn't gate on agent_network=true.
|
||||
|
||||
5. Performance
|
||||
- SynthesizeServices runs on every NetworkMap push. With N
|
||||
connected peers × M policies × K providers, what's the worst
|
||||
case allocation? Flag any O(N²) loop hidden in the synthesiser.
|
||||
- cloneInputFor allocates per middleware invocation. Estimate
|
||||
p99 request-time overhead under a 16-middleware chain.
|
||||
- Streaming response handling — the SSE parser must not buffer
|
||||
the whole stream.
|
||||
|
||||
6. Observability
|
||||
- Activity codes added for budget-rule decisions. Verify each
|
||||
has a UI label in the dashboard.
|
||||
- access-log fields (provider, model, tokens, cost, deny_code,
|
||||
agent_network flag) match what the dashboard renders.
|
||||
- Metrics surface: list every metric added in
|
||||
proxy/internal/metrics + proxy/internal/middleware/metrics.
|
||||
|
||||
7. Tests
|
||||
- For every "what to scrutinize" bullet, find the test that
|
||||
locks it down. If none exists, flag as a coverage gap.
|
||||
- Real-store coverage in management/agentnetwork (no mocks)
|
||||
is a stated goal — confirm and list any remaining mocks.
|
||||
- Bodytap truncation-replay test missing; Dispatcher
|
||||
timeout/panic tests missing; concurrent Budget exhaustion test
|
||||
missing — confirm these gaps from
|
||||
modules/30-proxy-middleware-framework.md.
|
||||
|
||||
OUT OF SCOPE — do not flag these as issues
|
||||
- Reaper / GC pass over stale synth services (scope-cut per AN-0)
|
||||
- URL-sync for tab state on AI Observability page
|
||||
- npm run lint broken on dashboard (pre-existing Next 16 issue)
|
||||
- Style nitpicks unless they affect correctness
|
||||
- Anything in MEMORY.md or claude-contexts/ paths
|
||||
|
||||
OUTPUT FORMAT (single markdown file)
|
||||
|
||||
# Agent review — agent-networks PR
|
||||
|
||||
## Summary
|
||||
- Modules reviewed: <list>
|
||||
- Findings: <N total — H high, M medium, L low>
|
||||
- Most concerning: <one-line>
|
||||
- Disagreements with the human review guides: <list or "none">
|
||||
|
||||
## Findings
|
||||
For each finding:
|
||||
### F<n>. <Short title> — <severity H/M/L> — module <ID>
|
||||
- **Where:** file:line (or file:line-range)
|
||||
- **What:** one sentence on the problem
|
||||
- **Why it matters:** consequence in production
|
||||
- **Evidence:** code snippet (≤20 lines) + reasoning
|
||||
- **Suggested fix:** one paragraph
|
||||
- **Test that would catch it:** suggest a test name + the assertion shape
|
||||
|
||||
## Gaps in the human review guides
|
||||
For every place the guide is wrong, stale, missing, or misleading:
|
||||
- **Guide:** modules/<file>.md section
|
||||
- **Issue:** <what the guide says vs reality>
|
||||
- **Suggested edit:** <one sentence>
|
||||
|
||||
## Confirmations
|
||||
List the high-stakes invariants you VERIFIED:
|
||||
- <invariant> — confirmed at file:line
|
||||
|
||||
## Methodology
|
||||
- Files actually read: <N>
|
||||
- Commits examined individually (git show): <list of SHAs>
|
||||
- Anything you didn't review and why: <list>
|
||||
|
||||
CONSTRAINTS
|
||||
- Read-only. Do not modify ANY file in either repo.
|
||||
- Statically-analyzable bash only — no $(), pipes, &&, or grep in
|
||||
Bash commands. Use Read for file content.
|
||||
- Use absolute paths (no cd).
|
||||
- Cite file:line for every concrete claim.
|
||||
- Severity rubric:
|
||||
H — could cause data loss, security boundary breach, runaway spend,
|
||||
or service outage in prod with realistic inputs.
|
||||
M — correctness bug under non-rare conditions, missing security
|
||||
control with a defense-in-depth replacement, or a perf issue
|
||||
that surfaces under typical load.
|
||||
L — code-smell, missing test, doc drift, minor inefficiency.
|
||||
- When in doubt about severity, write your reasoning; do not inflate.
|
||||
- The whole report must fit under 8000 words. If your draft is longer,
|
||||
cut the LOWEST-severity findings first.
|
||||
|
||||
When done, reply with a one-line confirmation including the absolute
|
||||
report path and the finding counts (H/M/L).
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## How to use this prompt
|
||||
|
||||
1. Spawn a second AI session (Claude Code, web Claude, or another agent
|
||||
with tool access). Make sure it can read both repos and write to
|
||||
`~/projects/claude-contexts/`.
|
||||
2. Paste the **Prompt** block above as the agent's first message.
|
||||
3. When the agent finishes, the report lands at
|
||||
`~/projects/claude-contexts/netbird/agent-networks-pr/AGENT_REVIEW_REPORT.md`.
|
||||
4. Diff the agent's findings against your own / your team's review notes
|
||||
— disagreements are often the most useful signal.
|
||||
5. Iterate: if the agent missed something, refine this prompt and re-run.
|
||||
|
||||
## Notes for the human running the prompt
|
||||
|
||||
- The prompt is intentionally bossy about scope and severity. Soft
|
||||
prompts produce padded reports.
|
||||
- The "Out of scope" list is the most-edited part: keep it in sync
|
||||
with what your team has agreed NOT to fix in this PR.
|
||||
- If you want a more shallow second-opinion run, drop the OUTPUT FORMAT
|
||||
section and ask for "your top 10 concerns in plain prose."
|
||||
- If you want a deeper run, add a second pass that asks the agent to
|
||||
rate every claim in the human review guides as `correct` / `wrong` /
|
||||
`unclear` with citations.
|
||||
@@ -1,34 +1,32 @@
|
||||
# agent-networks PR — review pack
|
||||
# Agent Networks — architecture documentation
|
||||
|
||||
A self-contained set of review documents for the agent-networks PR pair:
|
||||
`feature/agent-networks-backend` on this repo (netbird) and
|
||||
`feature/agent-networks` on the dashboard repo.
|
||||
A self-contained set of documents describing the agent-networks feature:
|
||||
an LLM-aware reverse-proxy middleware system plus account-level controls
|
||||
(budget rules, log collection toggles, PII redaction). The management
|
||||
server synthesises a per-peer middleware chain that the proxy executes on
|
||||
every LLM request.
|
||||
|
||||
## What to read first
|
||||
|
||||
1. **[00-overview.md](00-overview.md)** — the single entry point. PR
|
||||
scope, commit list, ownership matrix, risk hot-list, and links to
|
||||
every per-module guide.
|
||||
1. **[00-overview.md](00-overview.md)** — the single entry point. Feature
|
||||
scope, the module map, and the cross-cutting topics worth keeping in
|
||||
mind, with links to every per-module guide.
|
||||
2. **[01-end-to-end-flows.md](01-end-to-end-flows.md)** — three
|
||||
high-level mermaid diagrams: config-to-runtime synth/delivery,
|
||||
per-request lifecycle through the LLM chain, and the budget-rule
|
||||
feedback loop.
|
||||
3. **Per-module guides** under `modules/` — one file per
|
||||
review-able package. Each names its reviewer profile, lists commits
|
||||
in scope, shows file-level changes, includes its own flow diagrams,
|
||||
and calls out what to scrutinize.
|
||||
4. **[90-agent-review-prompt.md](90-agent-review-prompt.md)** — a
|
||||
self-contained prompt to feed to another AI agent (or a parallel
|
||||
reviewer) to perform structured code review against the same scope.
|
||||
3. **Per-module guides** under `modules/` — one file per package. Each
|
||||
describes the module boundary, the file-level layout, its own flow
|
||||
diagrams, the public contracts, the invariants it relies on, and the
|
||||
areas worth the closest attention.
|
||||
|
||||
## Directory layout
|
||||
|
||||
```
|
||||
docs/agent-networks/
|
||||
├── README.md # you are here
|
||||
├── 00-overview.md # PR summary + ownership matrix
|
||||
├── 00-overview.md # feature summary + module map
|
||||
├── 01-end-to-end-flows.md # cross-module mermaid diagrams
|
||||
├── 90-agent-review-prompt.md # prompt for an AI code reviewer
|
||||
└── modules/
|
||||
├── 10-shared-api.md # proto + OpenAPI wire contracts
|
||||
├── 20-management-store.md # SQL persistence layer
|
||||
@@ -38,47 +36,31 @@ docs/agent-networks/
|
||||
├── 31-proxy-middleware-builtin.md # 8 LLM-aware middlewares
|
||||
├── 32-proxy-llm-parsers.md # OpenAI/Anthropic/Bedrock SDKs + pricing
|
||||
├── 33-proxy-runtime.md # translate + serve + access-log
|
||||
├── 40-dashboard.md # UI for everything above (lives in the dashboard repo at feature/agent-networks)
|
||||
├── 40-dashboard.md # UI for everything above (lives in the dashboard repo)
|
||||
└── 50-path-routed-providers.md # Vertex AI + Bedrock (path-routed, keyfile:: creds, /bedrock prefix)
|
||||
```
|
||||
|
||||
The `40-dashboard.md` module documents code that lives in the **dashboard
|
||||
repo** (`feature/agent-networks` branch), not in this repo. The guide is
|
||||
co-located here so backend reviewers see the full picture in one place.
|
||||
repo**, not in this repo. The guide is co-located here so backend readers
|
||||
see the full picture in one place.
|
||||
|
||||
## How the per-module guides are structured
|
||||
|
||||
Every `modules/*.md` follows the same template so reviewers can scan a
|
||||
familiar shape:
|
||||
Every `modules/*.md` follows the same template so the docs are easy to
|
||||
scan:
|
||||
|
||||
- **Reviewer profile / time / risk / backward-compat impact** — fits in
|
||||
a single quote block at the top so triaging the file is a one-glance
|
||||
decision.
|
||||
- **Module boundary** — what this package owns within the PR; where it
|
||||
sits in the stack.
|
||||
- **Commits in scope** — pinned to the file scope, not the whole PR.
|
||||
- **Files changed** — path / status / LOC / role.
|
||||
- **Module boundary** — what this package owns; where it sits in the stack.
|
||||
- **Files** — path / role.
|
||||
- **Architecture & flow** — one or more mermaid diagrams.
|
||||
- **Public contracts** — function signatures, gRPC messages, JSON
|
||||
shapes, etc.
|
||||
- **Invariants** — semantic guarantees the module relies on or
|
||||
enforces.
|
||||
- **Public contracts** — function signatures, gRPC messages, JSON shapes.
|
||||
- **Invariants** — semantic guarantees the module relies on or enforces.
|
||||
- **Things to scrutinize** — split by correctness / security /
|
||||
concurrency / backward-compat / performance / observability.
|
||||
- **Test coverage** — every test file that locks down behavior in this
|
||||
- **Test coverage** — the test files that lock down behaviour in this
|
||||
module.
|
||||
- **Known limitations / non-goals** — what reviewers should NOT flag as
|
||||
bugs (out of scope on purpose).
|
||||
- **Known limitations / non-goals** — what is intentionally out of scope.
|
||||
- **Cross-references** — upstream/downstream module links + the
|
||||
end-to-end flow + the overview.
|
||||
|
||||
## Repos covered
|
||||
|
||||
- **Backend (this repo):** `feature/agent-networks-backend` —
|
||||
28 commits vs merge-base `14af17955`, ~28k net LOC added.
|
||||
- **Dashboard:** `feature/agent-networks` on
|
||||
`netbirdio/netbird-dashboard` — ~70 commits vs `main`, ~10k net LOC
|
||||
added.
|
||||
|
||||
See [00-overview.md](00-overview.md) for the full ownership matrix,
|
||||
commit roll-up, and cross-cutting risk hot-list.
|
||||
See [00-overview.md](00-overview.md) for the module map and the
|
||||
cross-cutting topics.
|
||||
|
||||
@@ -1,36 +1,24 @@
|
||||
# shared/api — wire contracts (proto + OpenAPI)
|
||||
|
||||
> **Reviewer profile:** API/contracts maintainer; protobuf field-number discipline + oapi-codegen experience; needs to recognise additive vs. breaking schema changes.
|
||||
> **Time to review:** 30-45 minutes
|
||||
> **Risk level:** Medium — wire-format surface that every other module pins against; backward-compat hinges on field-number discipline more than on logic correctness.
|
||||
> **Backward-compat impact:** Additive only (new proto fields use unallocated numbers, new RPCs default to `Unimplemented`, new OpenAPI schemas/paths are append-only; no existing field/RPC/schema removed or renumbered).
|
||||
|
||||
## Module boundary
|
||||
This module owns the cross-process contract surface between management, proxy, and dashboard. Two artefacts: `shared/management/proto/proxy_service.proto` (management↔proxy gRPC) and `shared/management/http/api/openapi.yml` (dashboard/CLI↔management REST). Both have generated companions checked in (`proxy_service.pb.go`, `proxy_service_grpc.pb.go`, `types.gen.go`) which must travel in lockstep with their sources. `shared/management/status/error.go` is in scope only for the four new typed `NotFound` constructors that the new HTTP handlers return.
|
||||
|
||||
Everything downstream — `management/agentnetwork`, `management/server/http/handlers/*`, `proxy/internal/*`, the dashboard SDK — consumes these types verbatim. A review here is about wire stability and codegen reproducibility, not behaviour: the behaviour reviews live in the management and proxy module guides.
|
||||
|
||||
## Commits in scope
|
||||
| SHA | Subject | LOC delta (this scope) |
|
||||
| --- | ------- | --------- |
|
||||
| 06ff17b38 | AN-0: additive proto + OpenAPI schemas + base types | +2374 / -774 (yml + types.gen.go regenerated) |
|
||||
| f810a4e35 | AN-1: store layer for providers/policies/guardrails/settings | +1 status/error.go (NotFound helpers — providers/policies/guardrails) |
|
||||
| a436b5fb3 | GC-0: account budget rule type + collection toggles + store CRUD | +1 status/error.go (`NewAgentNetworkBudgetRuleNotFoundError`) |
|
||||
| b22d5a181 | GC-2: enforce account budget rules as a min-wins ceiling | proto: `RecordLLMUsageRequest.group_ids = 8` |
|
||||
| 23bdf6871 | GC-4: HTTP API for budget rules + settings update | +293/+72 openapi.yml/types.gen.go (BudgetRule schemas, settings PUT) |
|
||||
| 468875cb4 | Wire EnableLogCollection to suppress access-log | proto: `PathTargetOptions.disable_access_log = 13` |
|
||||
Everything downstream — `management/agentnetwork`, `management/server/http/handlers/*`, `proxy/internal/*`, the dashboard SDK — consumes these types verbatim. The concern here is wire stability and codegen reproducibility, not behaviour: behaviour is covered in the management and proxy module guides.
|
||||
|
||||
`management.proto` and `signalexchange.proto` are unchanged. `status/error.go` only receives four additive constructors (lines 208-227); no existing error types are reshaped.
|
||||
|
||||
## Files changed
|
||||
| Path | Status | LOC | Role |
|
||||
| ---- | ------ | --- | ---- |
|
||||
| `shared/management/proto/proxy_service.proto` | Modified | +125 / -0 | Source of truth: 2 new RPCs, 1 new message group (`MiddlewareConfig` + slot enum), additive fields on `PathTargetOptions`, `AccessLog`, `RecordLLMUsageRequest` |
|
||||
| `shared/management/proto/proxy_service.pb.go` | Regenerated | +1932 / -1023 | protoc-gen-go v1.26.0 / protoc v7.34.1 output |
|
||||
| `shared/management/proto/proxy_service_grpc.pb.go` | Regenerated | +88 / -0 | Adds `CheckLLMPolicyLimits` + `RecordLLMUsage` client/server stubs and `UnimplementedProxyServiceServer` defaults |
|
||||
| `shared/management/http/api/openapi.yml` | Modified | +2371 / -... | 15 new `AgentNetwork*` schemas, 9 new path groups under `/api/agent-network/*` |
|
||||
| `shared/management/http/api/types.gen.go` | Regenerated | +504 / -0 | oapi-codegen v2.7.0 output (per commit msg; see codegen note below) |
|
||||
| `shared/management/status/error.go` | Modified | +20 / -0 | Four `NotFound` constructors for the new resource kinds (lines 208-227) |
|
||||
## Files
|
||||
| Path | Role |
|
||||
| ---- | ---- |
|
||||
| `shared/management/proto/proxy_service.proto` | Source of truth: 2 new RPCs, 1 new message group (`MiddlewareConfig` + slot enum), additive fields on `PathTargetOptions`, `AccessLog`, `RecordLLMUsageRequest` |
|
||||
| `shared/management/proto/proxy_service.pb.go` | Generated (protoc-gen-go) |
|
||||
| `shared/management/proto/proxy_service_grpc.pb.go` | Generated; adds `CheckLLMPolicyLimits` + `RecordLLMUsage` client/server stubs and `UnimplementedProxyServiceServer` defaults |
|
||||
| `shared/management/http/api/openapi.yml` | 15 new `AgentNetwork*` schemas, 9 new path groups under `/api/agent-network/*` |
|
||||
| `shared/management/http/api/types.gen.go` | Generated (oapi-codegen; see codegen note below) |
|
||||
| `shared/management/status/error.go` | Four `NotFound` constructors for the new resource kinds (lines 208-227) |
|
||||
|
||||
## Architecture & flow
|
||||
```mermaid
|
||||
@@ -54,21 +42,21 @@ sequenceDiagram
|
||||
|
||||
The proto changes split into three independent slices: (1) **mapping enrichment** — `PathTargetOptions` grows fields 8-13 so management can ship middleware configs, capture limits, and the agent-network / log-suppression flags down to the proxy without a second RPC; (2) **two new request/response RPCs** (`CheckLLMPolicyLimits`, `RecordLLMUsage`) for per-LLM-request budget arbitration; (3) **observability tag** — `AccessLog.agent_network` so management can route logs to the right surface.
|
||||
|
||||
The OpenAPI side is a thin CRUD surface — every resource (`Provider`, `Policy`, `Guardrail`, `BudgetRule`, `Settings`) follows the same `GET-list / POST / GET / PUT / DELETE` pattern, plus a read-only `/consumption` listing and a catalog endpoint. The `*Request` variants drop server-controlled fields (id, timestamps). `AgentNetworkBudgetRule` deliberately reuses `AgentNetworkPolicyLimits` to keep wire-shape parity with policies (GC-4 commit msg).
|
||||
The OpenAPI side is a thin CRUD surface — every resource (`Provider`, `Policy`, `Guardrail`, `BudgetRule`, `Settings`) follows the same `GET-list / POST / GET / PUT / DELETE` pattern, plus a read-only `/consumption` listing and a catalog endpoint. The `*Request` variants drop server-controlled fields (id, timestamps). `AgentNetworkBudgetRule` deliberately reuses `AgentNetworkPolicyLimits` to keep wire-shape parity with policies.
|
||||
|
||||
## Public contracts added
|
||||
- gRPC RPCs (`proxy_service.proto:52-57`): `CheckLLMPolicyLimits(CheckLLMPolicyLimitsRequest) → CheckLLMPolicyLimitsResponse`, `RecordLLMUsage(RecordLLMUsageRequest) → RecordLLMUsageResponse`. Both unary; default `UnimplementedProxyServiceServer` returns `codes.Unimplemented` (`proxy_service_grpc.pb.go:283-289`).
|
||||
- New messages (`proxy_service.proto:145-175,448-502`): `MiddlewareConfig`, `MiddlewareSlot` enum, `CheckLLMPolicyLimitsRequest`/`Response`, `RecordLLMUsageRequest`/`Response`.
|
||||
- New `PathTargetOptions` fields 8-13 (`proxy_service.proto:124-140`): `capture_max_request_bytes`, `capture_max_response_bytes`, `capture_content_types`, `middlewares`, `agent_network`, `disable_access_log`. All default-false / zero; pre-existing fields 1-7 byte-for-byte unchanged.
|
||||
- `AccessLog.agent_network = 18` (`proxy_service.proto:258-261`).
|
||||
- `RecordLLMUsageRequest.group_ids = 8` (`proxy_service.proto:496-498`) — added in GC-2 specifically so the record path can fan out to every applicable budget rule's window without a re-lookup.
|
||||
- `RecordLLMUsageRequest.group_ids = 8` (`proxy_service.proto:496-498`) — so the record path can fan out to every applicable budget rule's window without a re-lookup.
|
||||
- 15 new OpenAPI component schemas (`openapi.yml:5072-5829`): `AgentNetworkProvider[Request|Model]`, `AgentNetworkCatalog{Model,Provider,IdentityInjection,HeaderPairInjection,JSONMetadataInjection,ExtraHeader}`, `AgentNetworkPolicy[Request|TokenLimit|BudgetLimit|Limits]`, `AgentNetworkGuardrail[Checks|Request]`, `AgentNetworkConsumption`, `AgentNetworkSettings[Request]`, `AgentNetworkBudgetRule[Request]`.
|
||||
- 9 new path groups (`openapi.yml:12797-13460`): `/api/agent-network/{consumption,settings,budget-rules,budget-rules/{ruleId},catalog/providers,providers,providers/{providerId},policies,policies/{policyId},guardrails,guardrails/{guardrailId}}`.
|
||||
- Four typed NotFound errors (`shared/management/status/error.go:208-227`).
|
||||
|
||||
## Invariants
|
||||
- **Field-number monotonicity.** Every new proto field uses a previously-unallocated number in its message: `PathTargetOptions` 8-13 (was 1-7), `AccessLog` 18 (was 1-17), `RecordLLMUsageRequest` 8. `SendStatusUpdateRequest.inbound_listener = 50` (pre-existing) reserves 50+ for observability extensions, so 8 on `RecordLLMUsageRequest` doesn't conflict.
|
||||
- **Old proxies stay compatible.** Old management never sends `disable_access_log`/`middlewares`/`agent_network` (zero value → existing behaviour); old proxies that don't decode these fields just drop them silently (proto3 unknown-field semantics) — log emission stays on. AN-0's commit message explicitly claims "private-service proto/OpenAPI content is byte-for-byte untouched (0 deletions in proxy_service.proto and openapi.yml)" — verify by diff stat that no pre-existing field number changed.
|
||||
- **Old proxies stay compatible.** Old management never sends `disable_access_log`/`middlewares`/`agent_network` (zero value → existing behaviour); old proxies that don't decode these fields just drop them silently (proto3 unknown-field semantics) — log emission stays on. No pre-existing field number changed: the proto change is insertions only.
|
||||
- **Old management stays compatible.** The two new RPCs are registered on the same `management.ProxyService` descriptor; old proxies hitting them get `codes.Unimplemented` from the unimplemented embed (`proxy_service_grpc.pb.go:283-289`), which is the same fallback pattern `SyncMappings` already documents (`proxy_service.proto:20-21`).
|
||||
- **OpenAPI shapes are append-only.** New schemas are placed at the end of `components.schemas` (line 5072+); new paths at the end of `paths` (line 12797+). No existing schema's `required` list, enum, or property type was changed.
|
||||
- **`*Request` vs response asymmetry.** Read shapes (`AgentNetworkProvider`, `AgentNetworkPolicy`, `AgentNetworkGuardrail`, `AgentNetworkSettings`, `AgentNetworkBudgetRule`) require `created_at`/`updated_at`; the matching `*Request` shapes do not — server fills them. `AgentNetworkProviderRequest.api_key` is write-only (`openapi.yml:5158-5161` "never returned in responses"); reviewers should confirm the response schema (5072-5138) actually omits `api_key`.
|
||||
@@ -86,14 +74,14 @@ The OpenAPI side is a thin CRUD surface — every resource (`Provider`, `Policy`
|
||||
- Cluster + subdomain on `AgentNetworkSettings` are documented immutable (`openapi.yml:5686-5694`) and the `AgentNetworkSettingsRequest` (lines 5733-5752) doesn't accept them. Verify the `PUT /api/agent-network/settings` handler can't be tricked by extra JSON keys (oapi-codegen's `additionalProperties: false` is not declared here; spec defaults to permissive).
|
||||
|
||||
### Backward compatibility
|
||||
- The proto file claims field-number additivity; spot-check by diffing the merge-base proto against HEAD — every numbered field present at merge-base must still have the same name + type. The AN-0 commit message asserts "0 deletions in proxy_service.proto"; `git diff --stat` shows 125 insertions and 0 deletions, so this holds at the source-text level.
|
||||
- The proto change is field-number additive: every previously numbered field keeps the same name + type, and the change is insertions only (no deletions in `proxy_service.proto`), so this holds at the source-text level.
|
||||
- `proxy_service_grpc.pb.go` adds two RPC handlers and registers them in `ProxyService_ServiceDesc.Methods` (lines 543-552). The existing entries are unchanged and order-preserving — gRPC method dispatch is name-keyed, so order doesn't matter, but reviewing the diff (no method renamed/dropped) is still worth a glance.
|
||||
- OpenAPI 3.0 doesn't have a built-in deprecation flow for paths; if any client tooling iterates `paths.*`, the additive routes shouldn't break it, but generated SDKs (especially the dashboard's) need a regen to gain access to `AgentNetwork*`.
|
||||
|
||||
### Codegen pinning
|
||||
- `generate.sh` (`shared/management/http/api/generate.sh:14`) installs `oapi-codegen@latest`, yet the AN-0 commit message claims v2.7.0 was used. **This is a reproducibility gap** — re-running the script tomorrow may produce a different `types.gen.go`. Either pin the version in `generate.sh` (`@v2.7.0`) or document the pin in a `tools.go`.
|
||||
- proto codegen has the protoc version stamped in the generated file header (`proxy_service.pb.go:3-4` — protoc-gen-go v1.26.0 / protoc v7.34.1), matching the commit message. Good.
|
||||
- Reviewers should regenerate locally and confirm zero diff against the committed `types.gen.go` / `proxy_service.pb.go` before merge.
|
||||
- `generate.sh` (`shared/management/http/api/generate.sh:14`) installs `oapi-codegen@latest` rather than a pinned version. **This is a reproducibility gap** — re-running the script later may produce a different `types.gen.go`. Either pin the version in `generate.sh` (e.g. `@v2.7.0`) or document the pin in a `tools.go`.
|
||||
- proto codegen has the protoc / protoc-gen-go version stamped in the generated file header (`proxy_service.pb.go:3-4`).
|
||||
- Regenerate locally and confirm zero diff against the committed `types.gen.go` / `proxy_service.pb.go`.
|
||||
|
||||
## Test coverage
|
||||
| Test file | Locks down |
|
||||
@@ -109,9 +97,9 @@ Acceptable for codegen artefacts, but a single golden-file test that re-runs `oa
|
||||
- **No proto-side validation.** Numeric ranges (e.g. `window_seconds >= 60`, `cost_usd >= 0`, capture-byte clamps) are enforced in the OpenAPI schema via `minimum:` and inside Go code by the proxy/management, but `proto3` itself can't express them; downstream is expected to validate every message.
|
||||
- **`MiddlewareConfig.config_json` is `bytes`** (`proxy_service.proto:163`) — opaque to the proto layer. Schema validity is the middleware factory's problem. This is a deliberate tradeoff (per the comment at 161-162) but worth flagging: a corrupted/malicious config_json can only fail at proxy apply time, not at the wire-decode step.
|
||||
- **No catalog endpoint schema for the catalog itself** — the catalog data ships as a `GET /api/agent-network/catalog/providers` returning `[AgentNetworkCatalogProvider]` (`openapi.yml:13024`), but the catalog source-of-truth lives in `management/server/agentnetwork/catalog`, not here.
|
||||
- AN-0's commit message notes "Reaper design dropped per scope decision; not migrated." No reaper-related types appear in this scope — confirmed.
|
||||
- The reaper / GC design was cut from scope; no reaper-related types appear here.
|
||||
|
||||
## Cross-references
|
||||
- Downstream: [management/store](20-management-store.md), [management/agentnetwork](21-management-agentnetwork.md), [management/http handlers](22-management-http-handlers.md), [proxy/runtime](33-proxy-runtime.md)
|
||||
- Downstream: [management/store](20-management-store.md), [management/agentnetwork](21-management-agentnetwork.md), [management/handlers + wiring](22-management-handlers-wiring.md), [proxy/runtime](33-proxy-runtime.md)
|
||||
- End-to-end flow: [../01-end-to-end-flows.md](../01-end-to-end-flows.md)
|
||||
- Top-level: [../00-overview.md](../00-overview.md)
|
||||
|
||||
@@ -1,33 +1,23 @@
|
||||
# management/store — persistence for agent-network entities
|
||||
|
||||
> **Reviewer profile:** Storage maintainer; gorm + sqlite/postgres/mysql + AutoMigrate semantics, field-level encryption, upsert/`ON CONFLICT` correctness across engines.
|
||||
> **Time to review:** 35-45 minutes
|
||||
> **Risk level:** Medium — six brand-new tables behind AutoMigrate, one upsert-counter table that runs on the request hot path, and one column carrying an encrypted secret.
|
||||
> **Backward-compat impact:** Additive (six new tables created by AutoMigrate; the `Store` interface gains 23 methods, but no existing column/index is touched).
|
||||
|
||||
## Module boundary
|
||||
|
||||
This module is the persistence layer for the Agent Network feature. Everything the management server stores about LLM proxying — providers, policies, guardrails, the per-account settings row, a usage-counter table written on every proxied LLM request, and the GC-0 account-budget rules — flows through the methods added to `store.Store`. The module owns six tables, six entity types from `management/server/agentnetwork/types`, and a single hot-path upsert (`IncrementAgentNetworkConsumption`) consumed by the proxy fleet.
|
||||
This module is the persistence layer for the Agent Network feature. Everything the management server stores about LLM proxying — providers, policies, guardrails, the per-account settings row, a usage-counter table written on every proxied LLM request, and the account-budget rules — flows through the methods added to `store.Store`. The module owns six tables, six entity types from `management/server/agentnetwork/types`, and a single hot-path upsert (`IncrementAgentNetworkConsumption`) consumed by the proxy fleet.
|
||||
|
||||
Out of scope here: the catalog of provider definitions (compiled-in, no DB), the synthesizer/manager built on top of these CRUDs (covered in [21-management-agentnetwork.md](21-management-agentnetwork.md)), and the HTTP handlers that translate API requests into Save/Delete calls.
|
||||
|
||||
## Commits in scope
|
||||
## Files
|
||||
|
||||
| SHA | Subject | LOC delta |
|
||||
| --- | ------- | --------- |
|
||||
| f810a4e35 | AN-1: store layer for providers/policies/guardrails/settings | +552 |
|
||||
| 77b407632 | AN-2: agentnetwork module — adds `GetAllAgentNetworkProviders` + consumption methods to the store interface | +161 |
|
||||
| a436b5fb3 | GC-0: account budget rule type + collection toggles + store CRUD | +241/-1 |
|
||||
|
||||
## Files changed
|
||||
|
||||
| Path | Status | LOC | Role |
|
||||
| ---- | ------ | --- | ---- |
|
||||
| `management/server/store/sql_store_agentnetwork.go` | new | 466 | gorm implementations of all 23 store methods |
|
||||
| `management/server/store/sql_store_agentnetwork_budgetrule_test.go` | new | 112 | round-trip + account-scoping coverage against a real sqlite store |
|
||||
| `management/server/store/sql_store.go` | additive | +3 | one import, six entities appended to the `AutoMigrate` slice (sql_store.go:40, sql_store.go:141-142) |
|
||||
| `management/server/store/store.go` | additive | +26 | 23 methods added to the `Store` interface (store.go:328-354) |
|
||||
| `management/server/store/store_mock_agentnetwork.go` | new | 346 | mockgen output for the new interface surface |
|
||||
| Path | Role |
|
||||
| ---- | ---- |
|
||||
| `management/server/store/sql_store_agentnetwork.go` | gorm implementations of all 23 store methods |
|
||||
| `management/server/store/sql_store_agentnetwork_budgetrule_test.go` | round-trip + account-scoping coverage against a real sqlite store |
|
||||
| `management/server/store/sql_store.go` | one import, six entities appended to the `AutoMigrate` slice (sql_store.go:40, sql_store.go:141-142) |
|
||||
| `management/server/store/store.go` | 23 methods added to the `Store` interface (store.go:328-354) |
|
||||
| `management/server/store/store_mock_agentnetwork.go` | mockgen output for the new interface surface |
|
||||
|
||||
## Tables added / migrations
|
||||
|
||||
@@ -73,7 +63,7 @@ Reads decrypt provider secrets in-place; writes do `provider.Copy().EncryptSensi
|
||||
## Things to scrutinize
|
||||
|
||||
### Correctness
|
||||
- `SaveAgentNetworkProvider` saves the copy (sql_store_agentnetwork.go:95). The caller's in-memory pointer therefore keeps plaintext `api_key` and any `CreatedAt`/`UpdatedAt` gorm autofills land on the copy, not the original. Confirm with the AN-2 manager whether callers immediately re-fetch (they should, since timestamps don't sync back).
|
||||
- `SaveAgentNetworkProvider` saves the copy (sql_store_agentnetwork.go:95). The caller's in-memory pointer therefore keeps plaintext `api_key` and any `CreatedAt`/`UpdatedAt` gorm autofills land on the copy, not the original. Callers that need synced timestamps must re-fetch.
|
||||
- `IncrementAgentNetworkConsumption`'s `Create` provides initial counter values (`TokensInput: tokensIn`, etc.) in the row, and on conflict the assignments add the same deltas to the existing values. The insert-vs-update arithmetic is consistent. Cross-check that no engine in use (sqlite, postgres, mysql) silently rejects the `OnConflict` clause — GORM emits engine-specific SQL but `ON DUPLICATE KEY UPDATE` (mysql) vs `ON CONFLICT (...)` (sqlite/postgres) need their unique constraint to match the composite PK on `agent_network_consumption`; it does, by construction.
|
||||
- `IncrementAgentNetworkConsumption` writes `updated_at: time.Now().UTC()` literally inside the assignments map (sql_store_agentnetwork.go:333) — fine, but it's a Go-side timestamp captured at call time, not a DB-side `now()`. Acceptable for an audit field.
|
||||
- `GetAgentNetworkConsumption` returns a zero-valued non-nil row on `ErrRecordNotFound` (sql_store_agentnetwork.go:364-371). Document or rename — a typed sentinel error would be more orthodox; callers must know not to error-check.
|
||||
@@ -84,7 +74,7 @@ Reads decrypt provider secrets in-place; writes do `provider.Copy().EncryptSensi
|
||||
- `Save*Provider` uses `db.Save` on a struct with a PK already set — GORM emits UPDATE or INSERT based on row existence. No upsert clause is attached, so a race between two creates with the same generated `xid` (vanishingly unlikely) would surface as a PK violation.
|
||||
|
||||
### Migration safety
|
||||
- All six tables ride `AutoMigrate` (sql_store.go:141-142). AutoMigrate is additive: new columns get added, but it never drops columns nor narrows types. The GC-0 commit appends three `bool` columns to `agent_network_settings` (`EnableLogCollection`, `EnablePromptCollection`, `RedactPii`) — for existing rows these default to false at the GORM/DDL layer; the test at sql_store_agentnetwork_budgetrule_test.go:83-112 locks that down on a fresh sqlite. Verify postgres/mysql produce the same default.
|
||||
- All six tables ride `AutoMigrate` (sql_store.go:141-142). AutoMigrate is additive: new columns get added, but it never drops columns nor narrows types. Three `bool` columns on `agent_network_settings` (`EnableLogCollection`, `EnablePromptCollection`, `RedactPii`) default to false at the GORM/DDL layer for existing rows; the test at sql_store_agentnetwork_budgetrule_test.go:83-112 locks that down on a fresh sqlite. Verify postgres/mysql produce the same default.
|
||||
- The named index `idx_agent_network_settings_cluster_subdomain` on settings.go:15 is declared on only `subdomain`. Either the cluster column also needs `gorm:"index:idx_agent_network_settings_cluster_subdomain"` to make it composite, or the name is misleading.
|
||||
- The named index `idx_agent_network_provider` on `Provider.ProviderID` (provider.go:30) is *not* unique and not scoped to account — two providers in the same account with the same `provider_id` are permitted at the DB layer; uniqueness, if any, must live above the store.
|
||||
|
||||
@@ -103,7 +93,7 @@ Reads decrypt provider secrets in-place; writes do `provider.Copy().EncryptSensi
|
||||
| --------- | ---------- |
|
||||
| `sql_store_agentnetwork_budgetrule_test.go::TestAgentNetworkBudgetRule_RealStore_RoundTrip` | full save → reload of `AccountBudgetRule` including the JSON-serialised `PolicyLimits`, target slices, double-delete returns NotFound (lines 18-59) |
|
||||
| `sql_store_agentnetwork_budgetrule_test.go::TestAgentNetworkBudgetRule_RealStore_ScopedByAccount` | cross-account isolation for budget rules (lines 63-78) |
|
||||
| `sql_store_agentnetwork_budgetrule_test.go::TestAgentNetworkSettings_RealStore_CollectionTogglesRoundTrip` | GC-0 toggles default off, survive save/reload at the set values (lines 83-112) |
|
||||
| `sql_store_agentnetwork_budgetrule_test.go::TestAgentNetworkSettings_RealStore_CollectionTogglesRoundTrip` | collection toggles default off, survive save/reload at the set values (lines 83-112) |
|
||||
|
||||
Gap: there is no store-level test for providers (encryption round-trip), policies, guardrails, or `IncrementAgentNetworkConsumption` (concurrent upsert, window-key uniqueness). The consumption upsert is the most performance-sensitive method in this module and the only one without a real-sqlite test.
|
||||
|
||||
|
||||
@@ -1,59 +1,35 @@
|
||||
# management/agentnetwork — domain layer + synth pipeline
|
||||
|
||||
> **Reviewer profile:** Management server maintainer; familiar with the network-map controller, policy engine, gorm/SQLite store, Go concurrency (sync.Mutex + math/rand), and the reverse-proxy `service`/`proxy` packages + `ProxyMapping` proto shape.
|
||||
> **Time to review:** 180–240 minutes (largest module; budget for diagram cross-check + per-test triage).
|
||||
> **Risk level:** High — central business logic + budget enforcement + the source of every middleware-chain change the proxy executes.
|
||||
> **Backward-compat impact:** Additive within agent-network surface; one **behavioural break for opted-out accounts** in parser capture (capture flag now stamped explicitly false instead of being absent — see capture-pointer semantics below). Non-agent-network proxy services untouched (synth chain only ships on `agent-net-svc-*` targets).
|
||||
> **Backward-compat impact:** Additive within the agent-network surface; one **behavioural difference for opted-out accounts** in parser capture (the capture flag is stamped explicitly false instead of being absent — see capture-pointer semantics below). Non-agent-network proxy services are untouched (the synth chain only ships on `agent-net-svc-*` targets).
|
||||
|
||||
## Module boundary
|
||||
|
||||
`management/server/agentnetwork` owns every agent-network entity (providers, policies, guardrails, account budget rules, per-account settings, consumption rows) and **translates them into the in-memory `*rpservice.Service` that the reverse-proxy controller turns into `proto.ProxyMapping`s and pushes to clusters**. It is the *only* writer of the agent-network middleware chain.
|
||||
|
||||
Inside the package: `manager.go` is the CRUD + permissions-gated facade; `synthesizer.go` walks settings + providers + policies + guardrails and emits the per-account service plus every middleware's JSON config; `policyselect.go` runs per-request attribution (min-wins account ceiling, then "drain bigger pool first"); `reconcile.go` diffs successive synth outputs and emits precise Create/Update/Delete proxy-mapping updates plus a peer-map refresh. `labelgen/` mints DNS-safe subdomain labels; `catalog/` is the static provider catalogue; `types/` carries gorm entity structs. The `_realstack_test.go` files at the parent `management/server/` directory exercise the manager + network-map controller end-to-end with no mocks.
|
||||
Inside the package: `manager.go` is the CRUD + permissions-gated facade; `synthesizer.go` walks settings + providers + policies + guardrails and emits the per-account service plus every middleware's JSON config; `policyselect.go` runs per-request attribution (min-wins account ceiling, then "drain bigger pool first"); `reconcile.go` diffs successive synth outputs and emits precise Create/Update/Delete proxy-mapping updates plus a peer-map refresh. `labelgen/` mints DNS-safe subdomain labels; `catalog/` is the static provider catalogue; `types/` carries gorm entity structs. The `_realstack_test.go` files in the parent `management/server/` directory exercise the manager + network-map controller end-to-end with no mocks.
|
||||
|
||||
## Commits in scope
|
||||
## Files
|
||||
|
||||
| SHA | Subject | LOC delta |
|
||||
| --- | ------- | --------- |
|
||||
| `06ff17b38` | AN-0: additive base types | +804 |
|
||||
| `77b407632` | AN-2: agentnetwork module (manager, synth, catalog, policyselect) | +5049 |
|
||||
| `09e8059b6` | AN-2b: wire synth services into network map | +12/−13 |
|
||||
| `9ebe219fd` | test: lock auth→middleware group-propagation | +3 |
|
||||
| `665575932` | test: real-store coverage (no MockStore) | +174 |
|
||||
| `5adee2cb4` | no-mock provider-CRUD fan-out test | +212 |
|
||||
| `9ae476ea7` | no-mock baseline guards for enforcement + guardrail synth | +314 |
|
||||
| `a436b5fb3` | GC-0: budget rule type + collection toggles | +64 |
|
||||
| `5b408b0ef` | GC-1: budget-rule manager CRUD + settings update | +281 |
|
||||
| `b22d5a181` | GC-2: account budget rules as min-wins ceiling | +344/−32 |
|
||||
| `945f17f1a` | GC-3: account prompt-collection master switch | +108/−36 |
|
||||
| `23bdf6871` | GC-4: HTTP API surface (types-side delta) | +61/−5 |
|
||||
| `468875cb4` | EnableLogCollection → DisableAccessLog on synth target | +71 |
|
||||
| `7072f8125` | Account toggle = sole capture control + broader redact | +23/−17 |
|
||||
| `b438a7194` | Budget enforcement fix + extended PII redaction | +26/−13 |
|
||||
| `19e03d688` | Tests for redact-pii wiring across parsers | +97 |
|
||||
| `4836d5a19` | **Live-bug fix: gate `capture_prompt`/`capture_completion` on EnablePromptCollection** | +104/−14 |
|
||||
|
||||
## Files changed
|
||||
|
||||
| Path | Status | LOC | Role |
|
||||
| ---- | ------ | --- | ---- |
|
||||
| `agentnetwork/manager.go` | new | 788 | Manager interface + CRUD + permission gates + bootstrap-settings + reconcile trigger |
|
||||
| `agentnetwork/synthesizer.go` | new | 959 | Settings/policy → wire-format synthesis; sole writer of the proxy middleware chain |
|
||||
| `agentnetwork/policyselect.go` | new | 594 | Per-request policy attribution + account-budget ceiling (min-wins) |
|
||||
| `agentnetwork/reconcile.go` | new | 131 | Per-account synth diff vs in-memory cache → Create/Update/Delete |
|
||||
| `agentnetwork/catalog/catalog.go` | new | 685 | Static provider catalogue (auth headers, identity-injection shapes) |
|
||||
| `agentnetwork/labelgen/{labelgen,words}.go` | new | 202 | DNS-safe subdomain picker + curated wordlist |
|
||||
| `agentnetwork/types/provider.go` | new | 252 | Provider entity + APIKey + Models + ExtraValues + SessionKeys |
|
||||
| `agentnetwork/types/policy.go` | new | 192 | Policy entity + `PolicyLimits` (token + budget) |
|
||||
| `agentnetwork/types/guardrail.go` | new | 120 | Guardrail entity (`ModelAllowlist`, `PromptCapture`) |
|
||||
| `agentnetwork/types/budgetrule.go` | new | 106 | `AccountBudgetRule` (reuses `PolicyLimits`) |
|
||||
| `agentnetwork/types/settings.go` | new | 63 | Per-account `Settings` (Cluster, Subdomain, 3 toggles) |
|
||||
| `agentnetwork/types/consumption.go` | new | 58 | `Consumption` row + `WindowStart` aligner |
|
||||
| `agentnetwork/{synthesizer,policyselect,reconcile,wire_shape}_*test.go` | new | 2867 | See test coverage table |
|
||||
| `agentnetwork/types/consumption_test.go` | new | 141 | `WindowStart` alignment proofs |
|
||||
| `agentnetwork/labelgen/labelgen_test.go` | new | 101 | Deterministic picks + exhaustion + fallback |
|
||||
| `management/server/agentnetwork_realstack_test.go` | new | 212 | No-mock provider CRUD → network-map fan-out |
|
||||
| `management/server/agentnetwork_budgetrule_realstack_test.go` | new | 126 | No-mock budget-rule CRUD + settings preserve-immutable |
|
||||
| Path | Role |
|
||||
| ---- | ---- |
|
||||
| `agentnetwork/manager.go` | Manager interface + CRUD + permission gates + bootstrap-settings + reconcile trigger |
|
||||
| `agentnetwork/synthesizer.go` | Settings/policy → wire-format synthesis; sole writer of the proxy middleware chain |
|
||||
| `agentnetwork/policyselect.go` | Per-request policy attribution + account-budget ceiling (min-wins) |
|
||||
| `agentnetwork/reconcile.go` | Per-account synth diff vs in-memory cache → Create/Update/Delete |
|
||||
| `agentnetwork/catalog/catalog.go` | Static provider catalogue (auth headers, identity-injection shapes) |
|
||||
| `agentnetwork/labelgen/{labelgen,words}.go` | DNS-safe subdomain picker + curated wordlist |
|
||||
| `agentnetwork/types/provider.go` | Provider entity + APIKey + Models + ExtraValues + SessionKeys |
|
||||
| `agentnetwork/types/policy.go` | Policy entity + `PolicyLimits` (token + budget) |
|
||||
| `agentnetwork/types/guardrail.go` | Guardrail entity (`ModelAllowlist`, `PromptCapture`) |
|
||||
| `agentnetwork/types/budgetrule.go` | `AccountBudgetRule` (reuses `PolicyLimits`) |
|
||||
| `agentnetwork/types/settings.go` | Per-account `Settings` (Cluster, Subdomain, 3 toggles) |
|
||||
| `agentnetwork/types/consumption.go` | `Consumption` row + `WindowStart` aligner |
|
||||
| `agentnetwork/{synthesizer,policyselect,reconcile,wire_shape}_*test.go` | See test coverage table |
|
||||
| `agentnetwork/types/consumption_test.go` | `WindowStart` alignment proofs |
|
||||
| `agentnetwork/labelgen/labelgen_test.go` | Deterministic picks + exhaustion + fallback |
|
||||
| `management/server/agentnetwork_realstack_test.go` | No-mock provider CRUD → network-map fan-out |
|
||||
| `management/server/agentnetwork_budgetrule_realstack_test.go` | No-mock budget-rule CRUD + settings preserve-immutable |
|
||||
|
||||
## Architecture & flow
|
||||
|
||||
@@ -154,7 +130,7 @@ At request time the path is independent: the proxy calls `SelectPolicyForRequest
|
||||
|
||||
## Invariants
|
||||
|
||||
- **Min-wins / all-must-pass for account budget rules** (`checkAccountBudget`, `policyselect.go:353`): every applicable enabled rule is checked; first exhausted cap denies. Untargeted rules bind every caller. GC-2 contract.
|
||||
- **Min-wins / all-must-pass for account budget rules** (`checkAccountBudget`, `policyselect.go:353`): every applicable enabled rule is checked; first exhausted cap denies. Untargeted rules bind every caller.
|
||||
- **Account toggle is the SOLE control for capture enablement.** `applyAccountCollectionControls` (`synthesizer.go:701`) sets `merged.PromptCapture.Enabled = settings.EnablePromptCollection` *unconditionally*.
|
||||
- **Capture-pointer semantics on parser configs** — see "Things to scrutinize" below.
|
||||
- **`EnableLogCollection` ↔ `DisableAccessLog` is the only access-log toggle** (`synthesizer.go:770`). Default off ⇒ access log suppressed.
|
||||
@@ -168,11 +144,11 @@ At request time the path is independent: the proxy calls `SelectPolicyForRequest
|
||||
|
||||
### Correctness
|
||||
|
||||
- **Capture-pointer semantics — `*bool` vs `bool` (the live-bug fix at `4836d5a19`).** Three states, owned by separate sides:
|
||||
- **Capture-pointer semantics — `*bool` vs `bool`.** Three states, owned by separate sides:
|
||||
- **Wire JSON this module emits:** `buildParserConfigJSON` (`synthesizer.go:678-693`) *always* stamps the capture field. Agent-network targets ship `"capture_prompt": false` or `"capture_prompt": true` — never absent. Same for `"capture_completion"`. The happy-path test pins `{"capture_prompt":false}` (`synthesizer_test.go:174`).
|
||||
- **Proxy-side parser config (consumer):** parsers decode into `*bool`. Matrix:
|
||||
- `nil` (field absent) → **legacy default = emit**. Preserved for non-agent-network callers and pre-existing tests (the backward-compat hook).
|
||||
- `false` (field present, value false) → **suppress emission entirely**. New behaviour for opted-out agent-network accounts. Without this, `enable_log_collection=true` + `enable_prompt_collection=false` leaked raw user input AND raw model output to the access log (the live bug).
|
||||
- `false` (field present, value false) → **suppress emission entirely**. The behaviour for opted-out agent-network accounts. Without this, `enable_log_collection=true` + `enable_prompt_collection=false` would leak raw user input AND raw model output to the access log.
|
||||
- `true` → emit normally.
|
||||
- **Why the synth always stamps a value:** an agent-network mapping omitting the field would hit legacy "always emit" and re-introduce the leak. The `json.Marshal` error fallback at `synthesizer.go:687` degrades to `{}` — comment-claimed unreachable, but if ever fired re-introduces the leak. Consider fail-closed (return literal `{"capture_prompt":false}`) instead.
|
||||
- **`scoreCandidates` non-cumulative deny code.** Only the *last* exhausted policy's deny code survives (`policyselect.go:188-190`). Iteration order is store's natural order. Auth signal is `len(scored)==0`, so this is informational only — verify no UI depends on "first exhausted policy" semantics.
|
||||
@@ -182,7 +158,7 @@ At request time the path is independent: the proxy calls `SelectPolicyForRequest
|
||||
|
||||
### Security
|
||||
|
||||
- **Redact OR-merge:** merged `RedactPii` = account OR guardrail (`synthesizer.go:706`). **Parser-side flag is `settings.RedactPii` only, NOT the OR** — a guardrail-only opt-in does not propagate to parsers. Correct because the account toggle gates capture, but flag for the proxy reviewer.
|
||||
- **Redact OR-merge:** merged `RedactPii` = account OR guardrail (`synthesizer.go:706`). **Parser-side flag is `settings.RedactPii` only, NOT the OR** — a guardrail-only opt-in does not propagate to parsers. Correct because the account toggle gates capture, but worth noting on the proxy side.
|
||||
- **Group resolution must not leak across accounts.** Every store call carries `accountID` (`policyselect.go:73, 286, 298, 322, 334, 354`); `lowestIntersect` uses caller's claimed groups only (`policyselect.go:494`). Risk surface is upstream (handler populates `in.GroupIDs`).
|
||||
- **`UpdateSettings` preserves immutable Cluster + Subdomain** (`manager.go:558`). A client can't rebind the cluster.
|
||||
- **Provider session keypair backfill writes through `SaveAgentNetworkProvider`** (`synthesizer.go:256`) from a read-shaped call. Idempotent → worst case is a wasted write under concurrent reconcile + snapshot.
|
||||
@@ -218,19 +194,19 @@ At request time the path is independent: the proxy calls `SelectPolicyForRequest
|
||||
| Test file | Locks down |
|
||||
| --------- | ---------- |
|
||||
| `synthesizer_test.go` | Mock-store: `HappyPath` (8-mw chain ordering, `{"capture_prompt":false}` baseline); `No{Settings,Providers}`; `Disabled{Provider,Policy}_NoService`; `RouterConfigOrdering`; `PolicyCheckConfig_UnionsSourceGroups`; `OrphanProvider_HasEmptyAllowedGroups`; identity-inject for LiteLLM / Bifrost (overrides + partial disable) / Cloudflare / Portkey / Vercel / OpenRouter / generic non-customizable; `GuardrailMerge_AllowlistUnion_LimitsRestrictive`; `BackfillsMissingSessionKeys`; `HTTPUpstream_KeepsExplicitPort`; `UpstreamURLPath_FlowsToRouter`; `UnknownProviderID_FailsClosed`; `EmptyAPIKey_FailsClosed`. |
|
||||
| `synthesizer_realstore_test.go` | Real-sqlite: `SurvivesStatusToggle` reproduces the live 403 via disable/re-enable; `Reconcile_RealStore_PushesPrivateAfterStatusToggle` extends through reconcile push. |
|
||||
| `synthesizer_guardrail_realstore_test.go` | GC-3: `PromptCaptureAccountIsSoleControl`; `PromptCaptureFlowsWhenAccountOptsIn`; `AccountRedactWithoutGuardrailRedact`; `NoGuardrail_CaptureOff`. |
|
||||
| `synthesizer_realstore_test.go` | Real-sqlite: `SurvivesStatusToggle` reproduces the disable/re-enable 403 regression; `Reconcile_RealStore_PushesPrivateAfterStatusToggle` extends through reconcile push. |
|
||||
| `synthesizer_guardrail_realstore_test.go` | `PromptCaptureAccountIsSoleControl`; `PromptCaptureFlowsWhenAccountOptsIn`; `AccountRedactWithoutGuardrailRedact`; `NoGuardrail_CaptureOff`. |
|
||||
| `synthesizer_log_collection_realstore_test.go` | `LogCollection{Off_SuppressesAccessLog,On_PermitsAccessLog}` — verifies `DisableAccessLog` propagation through `ToProtoMapping`. |
|
||||
| `synthesizer_parser_redact_realstore_test.go` | **Live-bug regression suite (4836d5a19):** `ParserConfigsCarryRedactPii`; `ParserConfigsSuppressCaptureWhenLogCollectionOnly` (log=on/prompt=off ⇒ both capture flags false); `ParserConfigsOmitRedactPiiWhenOff`. |
|
||||
| `synthesizer_parser_redact_realstore_test.go` | **Capture-pointer regression suite:** `ParserConfigsCarryRedactPii`; `ParserConfigsSuppressCaptureWhenLogCollectionOnly` (log=on/prompt=off ⇒ both capture flags false); `ParserConfigsOmitRedactPiiWhenOff`. |
|
||||
| `policyselect_test.go` | Mock-store: `NoApplicablePolicies`; `AllowWithLowestGroupAttribution`; `LargerPoolWinsAcrossUsageLevels`; `StaysOnLargerPoolAfterPartialDrain`; `FallsThroughToSmallerPoolWhenLargerExhausted`; `TiebreakBy{LargerGroupPool,CreatedAt}`; `DeniesWhenAllExhausted`; `UncappedPolicyAlwaysWinsAgainstCapped`; `DisabledPolicyIgnored`; `StoreErrorPropagates`; `RejectsEmptyAccount`; `SharesGroupCounterAcrossPolicies`; `AntiFallThroughOnLowestGroup`; `BudgetOnlyExhaustionDenies`; `BudgetTighterThanTokenWins`. |
|
||||
| `policyselect_realstore_test.go` | Real-sqlite regression guard: `NoApplicablePolicies`; `AllowAndLowestGroupAttribution`; `LargerPoolWins_FallsThroughWhenExhausted`; `BudgetCapDenies`; `GroupCounterSharedAcrossPolicies`; `DisabledPolicyIgnored`. Must pass unchanged after GC-2. |
|
||||
| `policyselect_account_realstore_test.go` | GC-2: `AccountCeilingBindsEvenWithUncappedPolicy` (min-wins); `AccountGroupCeiling`; `AccountTargetUsersBindsOnlyThatUser`; `AccountRuleRecordsToOwnWindow`. |
|
||||
| `policyselect_realstore_test.go` | Real-sqlite regression guard: `NoApplicablePolicies`; `AllowAndLowestGroupAttribution`; `LargerPoolWins_FallsThroughWhenExhausted`; `BudgetCapDenies`; `GroupCounterSharedAcrossPolicies`; `DisabledPolicyIgnored`. |
|
||||
| `policyselect_account_realstore_test.go` | Account budget rules: `AccountCeilingBindsEvenWithUncappedPolicy` (min-wins); `AccountGroupCeiling`; `AccountTargetUsersBindsOnlyThatUser`; `AccountRuleRecordsToOwnWindow`. |
|
||||
| `reconcile_test.go` | `FirstSynth_EmitsCreate`; `NoChange_EmitsNothingExtra` (re-push as Modified — verify desired); `PolicyRemoved_EmitsDelete`; `NilProxyController_NoOp`; `EmptyAccountID_NoOp`; `ClusterFromMapping`. |
|
||||
| `wire_shape_test.go` | `TestSynthesizedService_WireShape` — proto-shape lockdown via `ToProtoMapping`. Catches "service not matching" (mapping reaches proxy but no SNI/HTTP route). Asserts ID, Domain, Mode, AuthToken, `Private`, `Auth.Oidc=false`, one path `/` + `https://noop.invalid/`, 8 middlewares with correct slot enums, router config `auth_header_value="Bearer sk-test-key"`. |
|
||||
| `labelgen/labelgen_test.go` | `PickUnique_{DeterministicWithSeededRng,AvoidsTakenWordsWhenMostAreReserved,FallsBackWhenAllReserved}`; `UniqueWords_DropsDuplicates`. |
|
||||
| `types/consumption_test.go` | `WindowStart_{AlignedToUnixEpoch,WithinWindowConverges,AcrossWindowsDiverges,DifferentWindowsHaveDifferentBuckets,SubMinuteAndMinuteAlignment,ZeroWindowReturnsInputUTC}`. Bucket alignment so multi-node reads converge. |
|
||||
| `agentnetwork_realstack_test.go` | `ProviderCRUD_FansOutToProxyAndClientPeers` — no-mock end-to-end through real account manager + network-map + agentnetwork: provider create propagates the updated map to both proxy peer and client peer with the synth DNS surface. |
|
||||
| `agentnetwork_budgetrule_realstack_test.go` | GC-1: `BudgetRuleCRUD_RealManager`; `UpdateSettings_PreservesImmutableAndTogglesCollection`. |
|
||||
| `agentnetwork_budgetrule_realstack_test.go` | `BudgetRuleCRUD_RealManager`; `UpdateSettings_PreservesImmutableAndTogglesCollection`. |
|
||||
|
||||
## Known limitations / explicit non-goals
|
||||
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
# management/handlers + wiring — HTTP API + gRPC delivery
|
||||
|
||||
> **Reviewer profile:** Management infra maintainer; comfortable with gorilla/mux REST handlers, the existing permissions/operations RBAC, the network_map controller fan-out, gRPC server-side streaming, and the reverse-proxy snapshot/live-update path.
|
||||
> **Time to review:** 60 minutes.
|
||||
> **Risk level:** Medium — surface is mostly additive, but two changes are load-bearing: `injectAllProxyPolicies` runs on every per-peer compute, and `shallowCloneMapping` now must round-trip `Private` (a missed field silently breaks every MODIFIED).
|
||||
> **Risk level:** Medium — the surface is mostly additive, but two changes are load-bearing: `injectAllProxyPolicies` runs on every per-peer compute, and `shallowCloneMapping` must round-trip `Private` (a missed field silently breaks every MODIFIED).
|
||||
> **Backward-compat impact:** Additive on the wire (new routes, new RPCs, new proto fields, new gorm column on `AccessLogEntry`). One management-internal break: `nbhttp.NewAPIHandler` gains a trailing `agentNetworkManager` parameter; `nil` is tolerated and silently skips route registration.
|
||||
|
||||
## Module boundary
|
||||
@@ -11,41 +9,29 @@ This module is the seam between the public Agent Network HTTP API and the proxy
|
||||
|
||||
South side: `ProxyServiceServer` (`proxy.go`) learns to (a) ship synth services to a proxy on initial snapshot, (b) resolve agent-network domains in `getServiceByDomain` for OIDC/session/tunnel-peer flows, (c) gate LLM requests via `CheckLLMPolicyLimits` + `RecordLLMUsage`, (d) preserve `Private` through `shallowCloneMapping` so per-proxy live updates don't silently flip services public. The network_map controller prepends synth services to `account.Services` on every per-peer compute; `accesslogentry.go` gains an indexed `AgentNetwork` column so the dashboard can filter cheaply.
|
||||
|
||||
## Commits in scope
|
||||
## Files
|
||||
|
||||
| SHA | Subject | LOC delta |
|
||||
| --- | ------- | --------- |
|
||||
| 09e8059b6 | AN-2b: wire synth into network map | controller.go +20 / repository.go +10 |
|
||||
| 9ecb6449d | AN-3: HTTP API handlers + routes | handlers/*, handler.go, module.go, codes.go |
|
||||
| 9a154714 | AN-6: access-log `agent_network` flag e2e | accesslogentry.go +5 |
|
||||
| 8cb9c187d | AN-7: enforcement + synth delivery | proxy.go +185, boot.go +24, service.go +109 |
|
||||
| 263dabd73 | preserve `Private` in clone | proxy.go +1, proxy_clone_test.go +59 |
|
||||
| 468875cb4 | wire `EnableLogCollection` suppression | service.go (DisableAccessLog plumbing) |
|
||||
| 23bdf6871 | GC-4: budget-rule + settings HTTP API | budget_handler.go +172, budget_handler_test.go +127 |
|
||||
|
||||
## Files changed
|
||||
|
||||
| Path | Status | LOC | Role |
|
||||
| ---- | ------ | --- | ---- |
|
||||
| `handlers/agentnetwork/providers_handler.go` | new | 216 | Catalog + provider CRUD + central `AddEndpoints` |
|
||||
| `handlers/agentnetwork/policies_handler.go` | new | 228 | Policy CRUD + shared `validatePolicy*` |
|
||||
| `handlers/agentnetwork/guardrails_handler.go` | new | 171 | Guardrail CRUD |
|
||||
| `handlers/agentnetwork/budget_handler.go` | new | 172 | Account-level budget rule CRUD |
|
||||
| `handlers/agentnetwork/settings_handler.go` | new | 74 | GET (200+`null` if unbootstrapped) + PUT toggles |
|
||||
| `handlers/agentnetwork/consumption_handler.go` | new | 53 | Read-only consumption rows |
|
||||
| `handlers/agentnetwork/handlers_test.go` | new | 239 | Real-store fixture; wire round-trip + validation |
|
||||
| `handlers/agentnetwork/budget_handler_test.go` | new | 127 | Budget-rule + settings toggles |
|
||||
| `server/http/handler.go` | edit | +7 | New `agentNetworkManager` arg; conditional `AddEndpoints` |
|
||||
| `server/permissions/modules/module.go` | edit | +2 | New `AgentNetwork` module key |
|
||||
| `internals/server/boot.go` | edit | +24 | Wires synthesiser adapter + limits service into proxy server |
|
||||
| `internals/server/modules.go` | edit | +12 | `AgentNetworkManager()` lazy-create node |
|
||||
| `internals/controllers/network_map/controller/controller.go` | edit | +20/-4 | `injectAllProxyPolicies` replaces 4 `InjectProxyPolicies` calls |
|
||||
| `internals/controllers/network_map/controller/repository.go` | edit | +10 | `SynthesizeAgentNetworkServices` repo method |
|
||||
| `internals/modules/reverseproxy/service/service.go` | edit | +109 | `MiddlewareConfig`, capture limits, `AgentNetwork`, `DisableAccessLog` + proto |
|
||||
| `internals/modules/reverseproxy/accesslogs/accesslogentry.go` | edit | +5 | Indexed `AgentNetwork bool` from proto |
|
||||
| `internals/shared/grpc/proxy.go` | edit | +185 | Synth wiring, 2 RPCs, domain fallback, `Private` in clone |
|
||||
| `internals/shared/grpc/proxy_clone_test.go` | new | 59 | Locks every `ProxyMapping` field minus `AuthToken` |
|
||||
| `server/activity/codes.go` | edit | +49 | 13 new activity codes (125-137) |
|
||||
| Path | Role |
|
||||
| ---- | ---- |
|
||||
| `handlers/agentnetwork/providers_handler.go` | Catalog + provider CRUD + central `AddEndpoints` |
|
||||
| `handlers/agentnetwork/policies_handler.go` | Policy CRUD + shared `validatePolicy*` |
|
||||
| `handlers/agentnetwork/guardrails_handler.go` | Guardrail CRUD |
|
||||
| `handlers/agentnetwork/budget_handler.go` | Account-level budget rule CRUD |
|
||||
| `handlers/agentnetwork/settings_handler.go` | GET (200+`null` if unbootstrapped) + PUT toggles |
|
||||
| `handlers/agentnetwork/consumption_handler.go` | Read-only consumption rows |
|
||||
| `handlers/agentnetwork/handlers_test.go` | Real-store fixture; wire round-trip + validation |
|
||||
| `handlers/agentnetwork/budget_handler_test.go` | Budget-rule + settings toggles |
|
||||
| `server/http/handler.go` | New `agentNetworkManager` arg; conditional `AddEndpoints` |
|
||||
| `server/permissions/modules/module.go` | New `AgentNetwork` module key |
|
||||
| `internals/server/boot.go` | Wires synthesiser adapter + limits service into proxy server |
|
||||
| `internals/server/modules.go` | `AgentNetworkManager()` lazy-create node |
|
||||
| `internals/controllers/network_map/controller/controller.go` | `injectAllProxyPolicies` replaces 4 `InjectProxyPolicies` calls |
|
||||
| `internals/controllers/network_map/controller/repository.go` | `SynthesizeAgentNetworkServices` repo method |
|
||||
| `internals/modules/reverseproxy/service/service.go` | `MiddlewareConfig`, capture limits, `AgentNetwork`, `DisableAccessLog` + proto |
|
||||
| `internals/modules/reverseproxy/accesslogs/accesslogentry.go` | Indexed `AgentNetwork bool` from proto |
|
||||
| `internals/shared/grpc/proxy.go` | Synth wiring, 2 RPCs, domain fallback, `Private` in clone |
|
||||
| `internals/shared/grpc/proxy_clone_test.go` | Locks every `ProxyMapping` field minus `AuthToken` |
|
||||
| `server/activity/codes.go` | 13 new activity codes (125-137) |
|
||||
|
||||
## HTTP routes added
|
||||
|
||||
@@ -153,7 +139,7 @@ End-to-end: HTTP write persists rows and emits an activity event; the manager th
|
||||
## Invariants
|
||||
|
||||
- **Synth services are never persisted.** Snapshot appends after `serviceManager.GetServicesForCluster` (`proxy.go:761-770`); network_map prepends before `InjectProxyPolicies` (`controller.go:117-126`).
|
||||
- **`shallowCloneMapping` must round-trip every `ProxyMapping` field except `AuthToken`** — `proxy_clone_test.go:50-58` enforces via `gproto.Equal`. The bug it guards (commit 263dabd73): missing `Private` made every MODIFIED arrive `private=false`, proxy skipped `ValidateTunnelPeer`, `UserGroups` stayed empty, llm_router denied `no_authorised_provider`; restart "fixed" because snapshot uses the original mapping.
|
||||
- **`shallowCloneMapping` must round-trip every `ProxyMapping` field except `AuthToken`** — `proxy_clone_test.go:50-58` enforces via `gproto.Equal`. The bug it guards: a missing `Private` made every MODIFIED arrive `private=false`, the proxy skipped `ValidateTunnelPeer`, `UserGroups` stayed empty, `llm_router` denied `no_authorised_provider`; a restart "fixed" it because the snapshot uses the original mapping.
|
||||
- **Limit-window floor is 60s** (`policies_handler.go:189-220`); enabled cap with both per-group and per-user at zero is rejected. Budget rules reuse the same validator (`budget_handler.go:170`).
|
||||
- **Manager is optional at boot.** `NewAPIHandler` registers routes only when non-nil (`handler.go:129`); `ProxyServiceServer` returns `Unimplemented` from both RPCs when limits service is unwired (`proxy.go:262-265, 306-309`).
|
||||
- **Settings GET on an unbootstrapped account returns 200 + `null`** (`settings_handler.go:65-72`) — not 404.
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
# proxy/middleware-framework — generic plugin system
|
||||
|
||||
> **Reviewer profile:** Proxy maintainer. Comfort with `net/http` handler composition, request/response interception, Go context propagation, `atomic.Pointer` snapshot patterns, and `sync` primitives. No LLM/agent-network domain knowledge required — every example built into this commit is generic.
|
||||
> **Time to review:** 60–75 minutes.
|
||||
> **Risk level:** **High** — every proxied request transits this chain. Budget exhaustion, panic recovery, or chain-close bugs hit the hot path for all targets, not just agent-network ones.
|
||||
> **Backward-compat impact:** Additive at the proxy. The `middleware` and `bodytap` packages are new (`proxy/internal/middleware/middleware.go:1`, `proxy/internal/middleware/bodytap/request.go:13`); existing proxy targets keep working until a chain is bound to them via `Manager.Rebuild`.
|
||||
|
||||
This module is the **framework only** — no LLM/agent-network domain knowledge is required, since every example built into it is generic.
|
||||
|
||||
## Module boundary
|
||||
|
||||
This module is the **framework only**: slots, chains, registry, dispatcher, accumulator, body-tap, output filters. No middleware *implementation* lives here — those land in `proxy/internal/middleware/builtin/*` (covered in module 31). The package contract is:
|
||||
@@ -15,37 +15,27 @@ This module is the **framework only**: slots, chains, registry, dispatcher, accu
|
||||
|
||||
Everything that crosses the framework boundary in either direction is value-typed and deep-copied — middlewares cannot mutate the live request directly, and the framework cannot inadvertently leak middleware-owned slices into the request hot path.
|
||||
|
||||
## Commits in scope
|
||||
## Files
|
||||
|
||||
| SHA | Subject | LOC delta |
|
||||
| --- | ------- | --------- |
|
||||
| `00bf0d328` | [agent-network] AN-4: proxy middleware framework | +2 740 (all 17 files in scope, this is the only commit that touches them) |
|
||||
|
||||
`git -C /Users/maycon/projects/netbird log --oneline 14af17955..HEAD -- proxy/internal/middleware/{chain,registry,manager,dispatcher,decision,headerpolicy,bodypolicy,keys,metadata,metrics,middleware,redaction,spec,types}.go proxy/internal/middleware/bodytap` returns exactly one commit. Subsequent AN-5/AN-7/GC-2 commits only add files under `builtin/` or touch wire paths — none modify the framework files in this module's scope. Review can be a single sit-down.
|
||||
|
||||
## Files changed
|
||||
|
||||
| Path | Status | LOC | Role |
|
||||
| ---- | ------ | --- | ---- |
|
||||
| `proxy/internal/middleware/middleware.go` | A | 47 | `Middleware` + `Factory` interfaces. |
|
||||
| `proxy/internal/middleware/types.go` | A | 242 | `Slot`, `FailMode`, `Decision`, all limit constants, `Input`/`Output`/`Mutations`/`UpstreamRewrite`/`AuthHeader` value types. |
|
||||
| `proxy/internal/middleware/spec.go` | A | 44 | Apply-time `Spec` (validated wire shape + runtime-injected fields) and `Clone`. |
|
||||
| `proxy/internal/middleware/registry.go` | A | 121 | `Registry` (factory map, RWMutex) and `Resolver` (Spec → bound `Middleware`). |
|
||||
| `proxy/internal/middleware/manager.go` | A | 412 | `Manager`, `chainTable` reverse index, `Rebuild`/`Invalidate*`, async chain close. |
|
||||
| `proxy/internal/middleware/chain.go` | A | 317 | `Chain.RunRequest`/`RunResponse`/`RunTerminal`, mutation gating, `cloneInputFor`. |
|
||||
| `proxy/internal/middleware/chain_test.go` | A | 330 | Metadata threading, LIFO response order, rewrite gating, UserGroups propagation, terminal accumulation. |
|
||||
| `proxy/internal/middleware/dispatcher.go` | A | 185 | Timeout/panic recovery, fail-mode, error classification, `filterOutput`. |
|
||||
| `proxy/internal/middleware/decision.go` | A | 80 | `RenderDenyResponse`, deny-code regex, status clamp. |
|
||||
| `proxy/internal/middleware/headerpolicy.go` | A | 69 | Compile-in header denylist + `FilterHeaderMutations`. |
|
||||
| `proxy/internal/middleware/bodypolicy.go` | A | 62 | `ValidateBodyReplace` / `ApplyBodyReplace` smuggling guards. |
|
||||
| `proxy/internal/middleware/keys.go` | A | 82 | Metadata key namespace constants. |
|
||||
| `proxy/internal/middleware/metadata.go` | A | 99 | `Accumulator` — allowlist, per-mw/per-request byte caps, redaction. |
|
||||
| `proxy/internal/middleware/metrics.go` | A | 171 | OTel instrument bundle (`proxy.middleware.*`). |
|
||||
| `proxy/internal/middleware/redaction.go` | A | 79 | `Scan` — PEM/JWT/AWS/bearer/Luhn-validated CC patterns. |
|
||||
| `proxy/internal/middleware/bodytap/request.go` | A | 229 | Capture + replay reader, `Budget` semaphore, bypass reason codes. |
|
||||
| `proxy/internal/middleware/bodytap/response.go` | A | 171 | `CapturingResponseWriter` (tee with `PassthroughWriter` for Flusher/Hijacker preservation). |
|
||||
|
||||
The 17 paths the scope listed all land in this single commit; no churn since.
|
||||
| Path | Role |
|
||||
| ---- | ---- |
|
||||
| `proxy/internal/middleware/middleware.go` | `Middleware` + `Factory` interfaces. |
|
||||
| `proxy/internal/middleware/types.go` | `Slot`, `FailMode`, `Decision`, all limit constants, `Input`/`Output`/`Mutations`/`UpstreamRewrite`/`AuthHeader` value types. |
|
||||
| `proxy/internal/middleware/spec.go` | Apply-time `Spec` (validated wire shape + runtime-injected fields) and `Clone`. |
|
||||
| `proxy/internal/middleware/registry.go` | `Registry` (factory map, RWMutex) and `Resolver` (Spec → bound `Middleware`). |
|
||||
| `proxy/internal/middleware/manager.go` | `Manager`, `chainTable` reverse index, `Rebuild`/`Invalidate*`, async chain close. |
|
||||
| `proxy/internal/middleware/chain.go` | `Chain.RunRequest`/`RunResponse`/`RunTerminal`, mutation gating, `cloneInputFor`. |
|
||||
| `proxy/internal/middleware/chain_test.go` | Metadata threading, LIFO response order, rewrite gating, UserGroups propagation, terminal accumulation. |
|
||||
| `proxy/internal/middleware/dispatcher.go` | Timeout/panic recovery, fail-mode, error classification, `filterOutput`. |
|
||||
| `proxy/internal/middleware/decision.go` | `RenderDenyResponse`, deny-code regex, status clamp. |
|
||||
| `proxy/internal/middleware/headerpolicy.go` | Compile-in header denylist + `FilterHeaderMutations`. |
|
||||
| `proxy/internal/middleware/bodypolicy.go` | `ValidateBodyReplace` / `ApplyBodyReplace` smuggling guards. |
|
||||
| `proxy/internal/middleware/keys.go` | Metadata key namespace constants. |
|
||||
| `proxy/internal/middleware/metadata.go` | `Accumulator` — allowlist, per-mw/per-request byte caps, redaction. |
|
||||
| `proxy/internal/middleware/metrics.go` | OTel instrument bundle (`proxy.middleware.*`). |
|
||||
| `proxy/internal/middleware/redaction.go` | `Scan` — PEM/JWT/AWS/bearer/Luhn-validated CC patterns. |
|
||||
| `proxy/internal/middleware/bodytap/request.go` | Capture + replay reader, `Budget` semaphore, bypass reason codes. |
|
||||
| `proxy/internal/middleware/bodytap/response.go` | `CapturingResponseWriter` (tee with `PassthroughWriter` for Flusher/Hijacker preservation). |
|
||||
|
||||
## Slot model
|
||||
|
||||
@@ -194,7 +184,7 @@ The framework explicitly aborts capture (and increments `proxy.middleware.captur
|
||||
| --------- | ---------- |
|
||||
| `proxy/internal/middleware/chain_test.go:77` | `RunRequest` threads metadata across on_request middlewares (regression for the "later mw can't see earlier mw's emissions" bug). |
|
||||
| `chain_test.go:110` | `RunResponse` reverse-order threading. |
|
||||
| `chain_test.go:142` | `cost_meter`-shaped scenario: response_parser registered after cost_meter still emits *before* cost_meter sees the bag (the exact `cost.skipped=missing_tokens` regression that prompted this commit). |
|
||||
| `chain_test.go:142` | `cost_meter`-shaped scenario: response_parser registered after cost_meter still emits *before* cost_meter sees the bag (guards the `cost.skipped=missing_tokens` regression). |
|
||||
| `chain_test.go:178` | `UpstreamRewrite` last-write-wins. |
|
||||
| `chain_test.go:206` | No middleware emits → nil rewrite. |
|
||||
| `chain_test.go:224` | Rewrite filtered when `CanMutate=false`. |
|
||||
|
||||
@@ -1,10 +1,8 @@
|
||||
# proxy/middleware-builtin — the LLM chain
|
||||
|
||||
> Reviewer profile: proxy / LLM. You should be comfortable with Go HTTP middleware
|
||||
> composition, OpenAI Responses API + Anthropic Messages API request/response shapes,
|
||||
> SSE framing, and the NetBird management gRPC surface (`proto.ProxyServiceClient`).
|
||||
> Spend most of your time on the **capture-pointer semantics** and the **limit_check ⇒
|
||||
> limit_record** record-once invariant — those carry the highest blast radius.
|
||||
The registry-mounted middleware set the proxy executes on every agent-network
|
||||
LLM request. The two highest-blast-radius areas are the **capture-pointer
|
||||
semantics** and the **limit_check ⇒ limit_record** record-once invariant.
|
||||
|
||||
Sibling module: [32-proxy-llm-parsers.md](./32-proxy-llm-parsers.md) — the SDK
|
||||
adapters + pricing catalog this chain delegates to.
|
||||
@@ -16,9 +14,9 @@ adapters + pricing catalog this chain delegates to.
|
||||
This module is the registry-mounted middleware set the proxy executes on
|
||||
every agent-network LLM request. Each sub-package registers itself via
|
||||
`init()`
|
||||
([builtin.go:32–34](../../../../netbird/proxy/internal/middleware/builtin/builtin.go));
|
||||
([builtin.go:32–34](../../../proxy/internal/middleware/builtin/builtin.go));
|
||||
the proxy server anonymous-imports the set
|
||||
([all_test.go:11–19](../../../../netbird/proxy/internal/middleware/builtin/all_test.go))
|
||||
([all_test.go:11–19](../../../proxy/internal/middleware/builtin/all_test.go))
|
||||
so the registry is populated at boot. The chain is wired by the management
|
||||
synthesiser and executed by the framework
|
||||
(`proxy/internal/middleware/{chain,dispatcher,accumulator}.go` — both out
|
||||
@@ -39,22 +37,10 @@ rewrites.
|
||||
| `cost_meter` | OnResponse | `llm.{provider,model}`, token buckets | `cost.usd_total` or `cost.skipped` | pricing lookup |
|
||||
| `llm_limit_record` | OnResponse | `llm.{attribution_group_id,attribution_window_seconds,input_tokens,output_tokens}`, `cost.usd_total` | none | gRPC `RecordLLMUsage` |
|
||||
|
||||
[all_test.go:26–40](../../../../netbird/proxy/internal/middleware/builtin/all_test.go)
|
||||
[all_test.go:26–40](../../../proxy/internal/middleware/builtin/all_test.go)
|
||||
locks the ID set; adding or removing one is a conscious extension.
|
||||
|
||||
## Commits in scope
|
||||
|
||||
`e64ea4b02` AN-5 built-in middlewares + registry · `a67022ffa` stamp parser
|
||||
id from catalog · `19d12adc8` openai match bare `/chat/completions` for
|
||||
Cloudflare AI Gateway · `afefa38ce` cached + cache-creation tokens ·
|
||||
`8cb9c187d` AN-7 enforcement + synth-service delivery · `b22d5a181` GC-2
|
||||
account budget rules min-wins · `7072f8125` account toggle sole control for
|
||||
prompt capture · `b1e66bca2` broader phone redactor · `b438a7194` budget
|
||||
enforcement fix + PII redaction across channels · `19e03d688` redact-pii
|
||||
wiring tests · `2a0d4991b` full-chain integration test · **`4836d5a19` gate
|
||||
prompt + completion capture on EnablePromptCollection (capture-pointer fix)**.
|
||||
|
||||
## Files changed
|
||||
## Files
|
||||
|
||||
| File | LOC | Notes |
|
||||
|---|---:|---|
|
||||
@@ -76,9 +62,9 @@ prompt + completion capture on EnablePromptCollection (capture-pointer fix)**.
|
||||
|
||||
Detects the LLM provider via `llm.DetectParser` (URL sniff) or by name via
|
||||
`llm.ParserByName` when synthesiser stamps `provider_id`
|
||||
([middleware.go:96–99](../../../../netbird/proxy/internal/middleware/builtin/llm_request_parser/middleware.go)).
|
||||
([middleware.go:96–99](../../../proxy/internal/middleware/builtin/llm_request_parser/middleware.go)).
|
||||
**Path-routed providers short-circuit first:** `parseVertexPath` and
|
||||
`parseBedrockPath` ([middleware.go:85–94](../../../../netbird/proxy/internal/middleware/builtin/llm_request_parser/middleware.go))
|
||||
`parseBedrockPath` ([middleware.go:85–94](../../../proxy/internal/middleware/builtin/llm_request_parser/middleware.go))
|
||||
pull the model + vendor out of the URL before parser selection runs — Vertex
|
||||
from `/v1/projects/.../publishers/{pub}/models/{model}:{action}` (publisher →
|
||||
vendor via `vertexPublisherVendor`), Bedrock from `/model/{id}/{action}` with
|
||||
@@ -89,27 +75,27 @@ grammar. For body-routed providers it decodes the body into `RequestFacts`
|
||||
`capture_prompt=true` (or absent — see capture-pointer semantics below) the
|
||||
prompt is run through `llm_guardrail.RedactPII` when `redact_pii=true` and
|
||||
truncated rune-safely to 3500 bytes
|
||||
([middleware.go:109–122](../../../../netbird/proxy/internal/middleware/builtin/llm_request_parser/middleware.go)).
|
||||
([middleware.go:109–122](../../../proxy/internal/middleware/builtin/llm_request_parser/middleware.go)).
|
||||
**Key invariant:** redaction is parser-side, not guardrail-side — access-log
|
||||
reads `llm.request_prompt_raw` directly.
|
||||
|
||||
### llm_router
|
||||
|
||||
Three-pass route selection in `matchRoute`
|
||||
([middleware.go:241–300](../../../../netbird/proxy/internal/middleware/builtin/llm_router/middleware.go)):
|
||||
([middleware.go:241–300](../../../proxy/internal/middleware/builtin/llm_router/middleware.go)):
|
||||
filter by `Models` claim → vendor-pin (a vendor-tagged request never crosses to
|
||||
another vendor's route) → filter by `AllowedGroupIDs` intersection → model
|
||||
precedence over path → tie-break by longest `UpstreamPath` prefix match.
|
||||
Model-miss returns `llm_policy.model_not_routable`; known-but-unauthorised
|
||||
returns `llm_policy.no_authorised_provider`. **Key invariant:** auth-header
|
||||
strip+inject rides on `UpstreamRewrite.{StripHeaders,AuthHeader}`
|
||||
([middleware.go:606–646](../../../../netbird/proxy/internal/middleware/builtin/llm_router/middleware.go))
|
||||
([middleware.go:606–646](../../../proxy/internal/middleware/builtin/llm_router/middleware.go))
|
||||
— NOT `HeadersAdd/HeadersRemove` — because the framework's mutation gate
|
||||
blocks `Authorization` on the generic header path.
|
||||
|
||||
**Path-routed providers route before the model table.** `Invoke` checks
|
||||
`isVertexPath` / `isBedrockPath`
|
||||
([middleware.go:138–216](../../../../netbird/proxy/internal/middleware/builtin/llm_router/middleware.go))
|
||||
([middleware.go:138–216](../../../proxy/internal/middleware/builtin/llm_router/middleware.go))
|
||||
ahead of the model lookup, so a path-carried model can't be claimed by a
|
||||
same-vendor body-routed provider. `matchPathRoute` enforces the route's `Models`
|
||||
allowlist (empty = catch-all) even though the model came from the URL.
|
||||
@@ -132,19 +118,19 @@ Full treatment in [50-path-routed-providers.md](./50-path-routed-providers.md).
|
||||
|
||||
Pre-flight gate. Reads `llm.resolved_provider_id`, calls
|
||||
`CheckLLMPolicyLimits` with a 2s context timeout
|
||||
([middleware.go:24, 97–106](../../../../netbird/proxy/internal/middleware/builtin/llm_limit_check/middleware.go)),
|
||||
([middleware.go:24, 97–106](../../../proxy/internal/middleware/builtin/llm_limit_check/middleware.go)),
|
||||
on allow stamps `llm.selected_policy_id`, `llm.attribution_group_id`,
|
||||
`llm.attribution_window_seconds`. **Key invariant:** fail-open. Nil
|
||||
`MgmtClient`, empty provider id, or RPC error returns `allowNoAttribution()`
|
||||
— management outage doesn't take down every LLM request. Operators audit via
|
||||
the access-log; PR3 may switch to fail-closed under a flag.
|
||||
the access-log; a future flag may switch this to fail-closed.
|
||||
|
||||
### llm_identity_inject
|
||||
|
||||
Dispatches per-rule between LiteLLM-shaped `HeaderPair`
|
||||
([middleware.go:169](../../../../netbird/proxy/internal/middleware/builtin/llm_identity_inject/middleware.go))
|
||||
([middleware.go:169](../../../proxy/internal/middleware/builtin/llm_identity_inject/middleware.go))
|
||||
and Portkey-shaped `JSONMetadata`
|
||||
([middleware.go:292](../../../../netbird/proxy/internal/middleware/builtin/llm_identity_inject/middleware.go)).
|
||||
([middleware.go:292](../../../proxy/internal/middleware/builtin/llm_identity_inject/middleware.go)).
|
||||
Identity is the peer's email (or `UserID` fallback); tags are the
|
||||
**authorising-groups intersection** emitted by `llm_router`, not the full
|
||||
`UserGroups` — a peer in 5 groups authorised under 1 only tags as that 1.
|
||||
@@ -160,20 +146,20 @@ Model allowlist deny + optional prompt-capture-with-redaction. Allowlist
|
||||
match is case-insensitive via `normaliseModel`; empty allowlist disables the
|
||||
check. Prompt capture reads `llm.request_prompt_raw` and emits
|
||||
`llm.request_prompt` only when `prompt_capture.enabled`
|
||||
([middleware.go:149–165](../../../../netbird/proxy/internal/middleware/builtin/llm_guardrail/middleware.go)).
|
||||
([middleware.go:149–165](../../../proxy/internal/middleware/builtin/llm_guardrail/middleware.go)).
|
||||
**Key invariant:** `RedactPII` is the exported function the parsers call —
|
||||
single PII contract across all three keys.
|
||||
|
||||
### llm_response_parser
|
||||
|
||||
Buffered and SSE paths share one `Invoke`
|
||||
([middleware.go:102–127](../../../../netbird/proxy/internal/middleware/builtin/llm_response_parser/middleware.go)):
|
||||
([middleware.go:102–127](../../../proxy/internal/middleware/builtin/llm_response_parser/middleware.go)):
|
||||
content-type sniffing dispatches to `invokeBuffered` (JSON, status<400) or
|
||||
`invokeStreaming` (text/event-stream, partial bodies tolerated). Streaming
|
||||
delegates to `accumulateStream`
|
||||
([streaming.go:21–30](../../../../netbird/proxy/internal/middleware/builtin/llm_response_parser/streaming.go))
|
||||
([streaming.go:21–30](../../../proxy/internal/middleware/builtin/llm_response_parser/streaming.go))
|
||||
using `llm.NewScanner`. A third path, `accumulateBedrockStream`
|
||||
([streaming_bedrock.go](../../../../netbird/proxy/internal/middleware/builtin/llm_response_parser/streaming_bedrock.go)),
|
||||
([streaming_bedrock.go](../../../proxy/internal/middleware/builtin/llm_response_parser/streaming_bedrock.go)),
|
||||
decodes the AWS binary event-stream (`application/vnd.amazon.eventstream`)
|
||||
returned by Bedrock's `-stream` actions — InvokeModel `chunk` frames wrap a
|
||||
base64 Anthropic event, Converse frames carry text + a trailing usage block.
|
||||
@@ -244,7 +230,7 @@ sequenceDiagram
|
||||
```
|
||||
|
||||
The integration test
|
||||
[agentnetwork_chain_integration_test.go](../../../../netbird/proxy/internal/middleware/builtin/agentnetwork_chain_integration_test.go)
|
||||
[agentnetwork_chain_integration_test.go](../../../proxy/internal/middleware/builtin/agentnetwork_chain_integration_test.go)
|
||||
exercises all three branches against a real sqlite store + bufconn gRPC —
|
||||
no mocks. Tests: `TestChain_AllowPath_StampsAttributionAndRecordsCounter`
|
||||
(line 130), `TestChain_DenyPath_GateRejectsAndNoConsumptionWritten` (line
|
||||
@@ -254,7 +240,7 @@ no mocks. Tests: `TestChain_AllowPath_StampsAttributionAndRecordsCounter`
|
||||
|
||||
| Middleware | Config shape |
|
||||
|---|---|
|
||||
| `llm_request_parser` | `{provider_id?, redact_pii?, capture_prompt?: *bool}` ([factory.go:19–37](../../../../netbird/proxy/internal/middleware/builtin/llm_request_parser/factory.go)) |
|
||||
| `llm_request_parser` | `{provider_id?, redact_pii?, capture_prompt?: *bool}` ([factory.go:19–37](../../../proxy/internal/middleware/builtin/llm_request_parser/factory.go)) |
|
||||
| `llm_router` | `{providers: [{id, models, upstream_scheme, upstream_host, upstream_path?, auth_header_name, auth_header_value, allowed_group_ids}]}` |
|
||||
| `llm_limit_check` | `{}` — pulls `MgmtClient` from `FactoryContext` |
|
||||
| `llm_identity_inject` | `{providers: [{provider_id, header_pair?|json_metadata?, extra_headers?}]}` |
|
||||
@@ -273,7 +259,7 @@ build time.
|
||||
stamps attribution metadata on the request leg; recorder reads it on the
|
||||
response leg. If a chain contains only the recorder, the
|
||||
skip-on-missing-attribution guard at
|
||||
[llm_limit_record/middleware.go:81–87, 98–103](../../../../netbird/proxy/internal/middleware/builtin/llm_limit_record/middleware.go)
|
||||
[llm_limit_record/middleware.go:81–87, 98–103](../../../proxy/internal/middleware/builtin/llm_limit_record/middleware.go)
|
||||
keeps counters consistent but no enforcement runs. Only-gate means
|
||||
counters never tick and headroom appears infinite.
|
||||
|
||||
@@ -282,10 +268,11 @@ build time.
|
||||
non-agent-network callers and pre-toggle tests). `false` = suppress the
|
||||
key entirely (access-log row carries zero prompt / completion content).
|
||||
`true` = emit. The synthesiser sets the pointer explicitly to the
|
||||
account's `EnablePromptCollection` toggle — see commit `4836d5a19` for
|
||||
the bug where a missing pointer was incorrectly treated as `false`. Fix
|
||||
in [llm_request_parser/factory.go:55–61](../../../../netbird/proxy/internal/middleware/builtin/llm_request_parser/factory.go)
|
||||
and the symmetric [llm_response_parser/middleware.go:62–68](../../../../netbird/proxy/internal/middleware/builtin/llm_response_parser/middleware.go).
|
||||
account's `EnablePromptCollection` toggle. The handling lives
|
||||
in [llm_request_parser/factory.go:55–61](../../../proxy/internal/middleware/builtin/llm_request_parser/factory.go)
|
||||
and the symmetric [llm_response_parser/middleware.go:62–68](../../../proxy/internal/middleware/builtin/llm_response_parser/middleware.go);
|
||||
a missing pointer must not be treated as `false` (that would suppress
|
||||
capture for legacy non-agent-network callers).
|
||||
`redact_pii` is an orthogonal `bool` controlling **form** of emitted
|
||||
content, not whether it's emitted.
|
||||
|
||||
@@ -314,22 +301,22 @@ build time.
|
||||
|
||||
**Correctness.** `llm_router` model match treats an empty `Models` slice as
|
||||
"claim every model"
|
||||
([middleware.go:238–248](../../../../netbird/proxy/internal/middleware/builtin/llm_router/middleware.go))
|
||||
([middleware.go:238–248](../../../proxy/internal/middleware/builtin/llm_router/middleware.go))
|
||||
for gateway-style providers — confirm no real provider record ships with an
|
||||
empty `Models` by accident. Path-prefix tie-break falls back to declaration
|
||||
order when no candidate prefix-matches, so the synthesiser must emit a
|
||||
deterministic order. `llm_limit_record` discards `strconv.ParseInt` errors
|
||||
([middleware.go:78–80](../../../../netbird/proxy/internal/middleware/builtin/llm_limit_record/middleware.go))
|
||||
([middleware.go:78–80](../../../proxy/internal/middleware/builtin/llm_limit_record/middleware.go))
|
||||
— relies on `llm_response_parser` always emitting parseable values; spot-check
|
||||
the streaming partial path on truncated bodies.
|
||||
|
||||
**Security.** Auth headers must NEVER appear on `Mutations.HeadersAdd/Remove`
|
||||
for the router — flag any PR that introduces a direct headers path. The
|
||||
capture-pointer regression (`4836d5a19`) is the kind of bug that ships PII to
|
||||
logs silently; walk every synthesiser config path and check the pointer is
|
||||
set explicitly. `llm_identity_inject` body inject silently skips on a
|
||||
for the router — a direct headers path would bypass the framework gate. The
|
||||
capture-pointer handling is the kind of place a bug ships PII to logs
|
||||
silently; every synthesiser config path must set the pointer explicitly.
|
||||
`llm_identity_inject` body inject silently skips on a
|
||||
non-object `metadata` field
|
||||
([middleware.go:262–270](../../../../netbird/proxy/internal/middleware/builtin/llm_identity_inject/middleware.go))
|
||||
([middleware.go:262–270](../../../proxy/internal/middleware/builtin/llm_identity_inject/middleware.go))
|
||||
— header path still attributes, but body-level tag-budget enforcement
|
||||
doesn't run for that request.
|
||||
|
||||
@@ -344,7 +331,7 @@ is O(1); SSE accumulation is single-pass. No map allocation per call.
|
||||
**Observability.** Every deny stamps `llm_policy.decision=deny` and a
|
||||
matching `llm_policy.reason` — access-log can pivot on either.
|
||||
`llm_limit_record` only logs at `Debugf` on RPC failure
|
||||
([middleware.go:125–130](../../../../netbird/proxy/internal/middleware/builtin/llm_limit_record/middleware.go));
|
||||
([middleware.go:125–130](../../../proxy/internal/middleware/builtin/llm_limit_record/middleware.go));
|
||||
operators need an alternate signal (metric on `RecordLLMUsage` failures) for
|
||||
counter accuracy.
|
||||
|
||||
|
||||
@@ -1,12 +1,12 @@
|
||||
# proxy/llm-parsers — SDK adapters + pricing + SSE
|
||||
|
||||
> Reviewer profile: LLM / proxy. You should know the OpenAI Responses API
|
||||
> (`/v1/responses`) and the older Chat Completions API (`/v1/chat/completions`),
|
||||
> the Anthropic Messages API (`/v1/messages`), the SSE wire format
|
||||
> (`event:` / `data:` lines, `\n\n` framing, CRLF tolerance), and the difference
|
||||
> between OpenAI's cached-prompt **subset** vs Anthropic's cache_read **additive**
|
||||
> token accounting. The pricing table's per-provider cost formula is the
|
||||
> highest-leverage place a small bug would silently mis-bill operators.
|
||||
The runtime-agnostic LLM library: the OpenAI Responses API (`/v1/responses`)
|
||||
and the older Chat Completions API (`/v1/chat/completions`), the Anthropic
|
||||
Messages API (`/v1/messages`), the SSE wire format (`event:` / `data:` lines,
|
||||
`\n\n` framing, CRLF tolerance), and per-provider token accounting (OpenAI's
|
||||
cached-prompt **subset** vs Anthropic's cache_read **additive** model). The
|
||||
pricing table's per-provider cost formula is the highest-leverage place a
|
||||
small bug would silently mis-bill operators.
|
||||
|
||||
Sibling module: [31-proxy-middleware-builtin.md](./31-proxy-middleware-builtin.md)
|
||||
— the 8 middlewares that consume this package's parsers + pricing loader.
|
||||
@@ -28,20 +28,11 @@ proxy-framework dependencies:
|
||||
symlink-safe Unix loader (build-tagged stub elsewhere).
|
||||
- `fixtures/` — captured request/response/stream bodies the tests replay.
|
||||
|
||||
Lifted out of the legacy plugin runtime by commit `8efbd430d` so the same
|
||||
parsers can be reused later by a WASM adapter
|
||||
([parser.go:1–6](../../../../netbird/proxy/internal/llm/parser.go)).
|
||||
The package carries zero proxy-framework dependencies so the same parsers can
|
||||
be reused later by a WASM adapter
|
||||
([parser.go:1–6](../../../proxy/internal/llm/parser.go)).
|
||||
|
||||
## Commits in scope
|
||||
|
||||
`8efbd430d` G4 part 1: lift LLM utils + delete legacy plugin runtime ·
|
||||
`06ff17b38` AN-0: additive proto + OpenAPI schemas + base types ·
|
||||
`19d12adc8` openai match bare `/chat/completions` for Cloudflare AI Gateway ·
|
||||
`afefa38ce` cached + cache-creation tokens for OpenAI and Anthropic ·
|
||||
`a67022ffa` stamp parser id from catalog into `llm_request_parser`.
|
||||
Downstream-consumer commits are covered in the sibling module review.
|
||||
|
||||
## Files changed
|
||||
## Files
|
||||
|
||||
| File | LOC | Notes |
|
||||
|---|---:|---|
|
||||
@@ -84,15 +75,15 @@ flowchart TD
|
||||
```
|
||||
|
||||
OpenAI's URL hints
|
||||
([openai.go:27–33](../../../../netbird/proxy/internal/llm/openai.go)) include
|
||||
([openai.go:27–33](../../../proxy/internal/llm/openai.go)) include
|
||||
both `/v1/chat/completions` and the bare `/chat/completions` — the latter
|
||||
covers Cloudflare AI Gateway, which rewrites the canonical version segment
|
||||
(commit `19d12adc8`). Anthropic's hints are `/v1/messages` and `/v1/complete`
|
||||
([anthropic.go:14–17](../../../../netbird/proxy/internal/llm/anthropic.go)).
|
||||
covers Cloudflare AI Gateway, which rewrites the canonical version segment.
|
||||
Anthropic's hints are `/v1/messages` and `/v1/complete`
|
||||
([anthropic.go:14–17](../../../proxy/internal/llm/anthropic.go)).
|
||||
Both implementations use case-insensitive substring matching so a proxy prefix
|
||||
strip / rewrite doesn't defeat detection.
|
||||
|
||||
`ParserByName` ([parser.go:93–103](../../../../netbird/proxy/internal/llm/parser.go))
|
||||
`ParserByName` ([parser.go:93–103](../../../proxy/internal/llm/parser.go))
|
||||
is the **agent-network bypass**: the synthesiser knows which parser to use
|
||||
because it built the synth service from the catalog, so it stamps
|
||||
`provider_id` on the parser config and the middleware skips URL sniffing
|
||||
@@ -132,12 +123,12 @@ sequenceDiagram
|
||||
```
|
||||
|
||||
`Scanner.Next`
|
||||
([sse.go:44–87](../../../../netbird/proxy/internal/llm/sse.go)) returns one
|
||||
([sse.go:44–87](../../../proxy/internal/llm/sse.go)) returns one
|
||||
event per `\n\n` boundary; multiple `data:` lines join with `\n`; comment lines
|
||||
(starting with `:`) are skipped per the SSE spec; a trailing event without a
|
||||
closing blank line is still returned before `io.EOF` so a server that closes
|
||||
the connection cleanly doesn't lose the last frame
|
||||
([sse.go:55–58](../../../../netbird/proxy/internal/llm/sse.go)). CRLF is
|
||||
([sse.go:55–58](../../../proxy/internal/llm/sse.go)). CRLF is
|
||||
normalised in `trimEOL` so fixtures captured from live servers replay
|
||||
unchanged.
|
||||
|
||||
@@ -145,13 +136,13 @@ unchanged.
|
||||
|
||||
### OpenAI
|
||||
|
||||
[openai.go:54–67](../../../../netbird/proxy/internal/llm/openai.go) defines
|
||||
[openai.go:54–67](../../../proxy/internal/llm/openai.go) defines
|
||||
`openAIRequest` with three prompt fields: `messages` (Chat Completions),
|
||||
`prompt` (legacy), `input` (Responses API). The decoder uses
|
||||
`json.RawMessage` so each shape is parsed lazily.
|
||||
|
||||
`ParseResponse`
|
||||
([openai.go:117–146](../../../../netbird/proxy/internal/llm/openai.go))
|
||||
([openai.go:117–146](../../../proxy/internal/llm/openai.go))
|
||||
accepts both naming conventions: Chat Completions returns
|
||||
`prompt_tokens`/`completion_tokens`, Responses API returns
|
||||
`input_tokens`/`output_tokens`. `pickInt64` prefers Responses-API names and
|
||||
@@ -165,13 +156,13 @@ responses where `cached > total`.
|
||||
|
||||
### Anthropic
|
||||
|
||||
[anthropic.go:37–49](../../../../netbird/proxy/internal/llm/anthropic.go)
|
||||
[anthropic.go:37–49](../../../proxy/internal/llm/anthropic.go)
|
||||
defines `anthropicRequest` covering Messages API (`system` + `messages[]`)
|
||||
and legacy `/v1/complete` (`prompt` string). `ExtractPrompt` emits
|
||||
`system: <text>` first when present, then per-message `role: content`.
|
||||
|
||||
`ParseResponse`
|
||||
([anthropic.go:82–104](../../../../netbird/proxy/internal/llm/anthropic.go))
|
||||
([anthropic.go:82–104](../../../proxy/internal/llm/anthropic.go))
|
||||
fills three independent token buckets: `InputTokens`, `CacheReadInputTokens`,
|
||||
`CacheCreationInputTokens`. Latter two are **additive** (not subset).
|
||||
`TotalTokens` sums all four so downstream dashboards render one "tokens"
|
||||
@@ -182,7 +173,7 @@ non-empty text with newlines, falling back to legacy `completion`.
|
||||
|
||||
### Bedrock
|
||||
|
||||
[bedrock.go](../../../../netbird/proxy/internal/llm/bedrock.go) implements the
|
||||
[bedrock.go](../../../proxy/internal/llm/bedrock.go) implements the
|
||||
`Parser` interface for the AWS Bedrock runtime. Bedrock is **path-routed**: the
|
||||
model lives in the URL (`/model/{id}/{action}`), so the request middleware
|
||||
extracts it (see [50-path-routed-providers.md](./50-path-routed-providers.md))
|
||||
@@ -207,17 +198,17 @@ AWS binary event-stream content-type (`application/vnd.amazon.eventstream`,
|
||||
|
||||
`Scanner` is `bufio`-backed, 64 KiB read buffer, 1 MiB max line so a
|
||||
malicious upstream can't blow process memory
|
||||
([sse.go:33–38, 97–100](../../../../netbird/proxy/internal/llm/sse.go)).
|
||||
([sse.go:33–38, 97–100](../../../proxy/internal/llm/sse.go)).
|
||||
`splitField` strips one space after the `:` per the SSE spec. Documented
|
||||
`not safe for concurrent use`; every consumer creates a fresh scanner per
|
||||
response body. Streaming accumulators live in the middleware package
|
||||
([llm_response_parser/streaming.go](../../../../netbird/proxy/internal/middleware/builtin/llm_response_parser/streaming.go))
|
||||
([llm_response_parser/streaming.go](../../../proxy/internal/middleware/builtin/llm_response_parser/streaming.go))
|
||||
but use `llm.NewScanner` so the framing contract stays here.
|
||||
|
||||
### Pricing catalog
|
||||
|
||||
`Table.Cost`
|
||||
([pricing.go:129–174](../../../../netbird/proxy/internal/llm/pricing/pricing.go))
|
||||
([pricing.go:129–174](../../../proxy/internal/llm/pricing/pricing.go))
|
||||
is the cost formula — most security-relevant math in this module:
|
||||
|
||||
| Provider | Formula |
|
||||
@@ -227,7 +218,7 @@ is the cost formula — most security-relevant math in this module:
|
||||
| default | `inTokens × InputPer1K + outTokens × OutputPer1K` |
|
||||
|
||||
`bedrock` shares the Anthropic additive-cache formula
|
||||
([pricing.go:172-174](../../../../netbird/proxy/internal/llm/pricing/pricing.go)):
|
||||
([pricing.go:172-174](../../../proxy/internal/llm/pricing/pricing.go)):
|
||||
Anthropic-on-Bedrock reports the same additive cache buckets, while non-Anthropic
|
||||
Bedrock models (Nova, Llama) simply report zero in those buckets so cost reduces
|
||||
to `input + output`.
|
||||
@@ -236,7 +227,7 @@ Each per-bucket rate falls back to `InputPer1K` when zero — operators opt in
|
||||
to discounts by setting the field.
|
||||
|
||||
`Loader`
|
||||
([pricing.go:212–268](../../../../netbird/proxy/internal/llm/pricing/pricing.go))
|
||||
([pricing.go:212–268](../../../proxy/internal/llm/pricing/pricing.go))
|
||||
overlays an optional `pricing.yaml` from data-dir on top of the go:embed
|
||||
defaults. Atomic pointer swap means readers never observe a partial update.
|
||||
The mtime-poll reloader (30s default cadence) keeps the previous table on
|
||||
@@ -248,7 +239,7 @@ Operator overrides only carry the entries they want to change.
|
||||
## Public contracts
|
||||
|
||||
**`Parser` interface**
|
||||
([parser.go:50–66](../../../../netbird/proxy/internal/llm/parser.go)):
|
||||
([parser.go:50–66](../../../proxy/internal/llm/parser.go)):
|
||||
|
||||
```go
|
||||
type Parser interface {
|
||||
@@ -263,25 +254,25 @@ type Parser interface {
|
||||
```
|
||||
|
||||
Adding a provider means implementing this interface and appending to the
|
||||
slice returned by `Parsers()` ([parser.go:78–84](../../../../netbird/proxy/internal/llm/parser.go)).
|
||||
slice returned by `Parsers()` ([parser.go:78–84](../../../proxy/internal/llm/parser.go)).
|
||||
Order matters: `DetectFromURL` ties resolve by registration order.
|
||||
`Parsers()` today returns `{OpenAIParser, AnthropicParser, BedrockParser}`.
|
||||
|
||||
**`ProviderID` enum**
|
||||
([parser.go:10–19](../../../../netbird/proxy/internal/llm/parser.go)):
|
||||
**`Provider` enum**
|
||||
([parser.go:8–18](../../../proxy/internal/llm/parser.go)):
|
||||
`ProviderUnknown = 0`, `ProviderOpenAI = 1`, `ProviderAnthropic = 2`,
|
||||
`ProviderBedrock = 3`. Numeric values are persisted in nothing today but treat
|
||||
them as wire-stable — new providers must take fresh numbers.
|
||||
|
||||
**`Pricing` lookup**
|
||||
([pricing.go:129](../../../../netbird/proxy/internal/llm/pricing/pricing.go)):
|
||||
([pricing.go:129](../../../proxy/internal/llm/pricing/pricing.go)):
|
||||
|
||||
```go
|
||||
func (t *Table) Cost(provider, model string, inTokens, outTokens, cachedInput, cacheCreation int64) (float64, bool)
|
||||
```
|
||||
|
||||
Nil-safe: `t.Cost` on a nil receiver returns `(0, false)`
|
||||
([pricing.go:130–132](../../../../netbird/proxy/internal/llm/pricing/pricing.go)).
|
||||
([pricing.go:130–132](../../../proxy/internal/llm/pricing/pricing.go)).
|
||||
`ok=false` means provider or model is absent from the loaded table; the caller
|
||||
emits `cost.skipped=unknown_model`.
|
||||
|
||||
@@ -291,7 +282,7 @@ emits `cost.skipped=unknown_model`.
|
||||
functional `loadPricing` (uses `syscall.O_NOFOLLOW` and `f.Stat()` on an
|
||||
open descriptor — both Unix-only). `pricing_other.go` is a build-tag
|
||||
fallback that returns `"not supported on this platform"`
|
||||
([pricing_other.go:14–16](../../../../netbird/proxy/internal/llm/pricing/pricing_other.go)).
|
||||
([pricing_other.go:14–16](../../../proxy/internal/llm/pricing/pricing_other.go)).
|
||||
The proxy is Linux-only in production today; a Windows port needs an
|
||||
equivalent path-as-handle implementation. Reviewers building on Windows
|
||||
should expect this surface to return an error at startup if an override
|
||||
@@ -299,23 +290,23 @@ emits `cost.skipped=unknown_model`.
|
||||
|
||||
2. **SSE scanner handles partial chunks.** A buffered prefix that doesn't end
|
||||
in `\n\n` still yields its accumulated event before `io.EOF`
|
||||
([sse.go:55–58](../../../../netbird/proxy/internal/llm/sse.go)). Tests:
|
||||
([sse.go:55–58](../../../proxy/internal/llm/sse.go)). Tests:
|
||||
`TestSSEScanner_OpenAIFixture`, `TestSSEScanner_AnthropicFixture`,
|
||||
`TestSSEScanner_MultilineData`, `TestSSEScanner_CRLF`. The streaming
|
||||
accumulators ride on this: `accumulateAnthropicStream` and
|
||||
`accumulateOpenAIStream` `break` on any scanner error to return partial
|
||||
usage rather than aborting
|
||||
([streaming.go:68–73, 144–150](../../../../netbird/proxy/internal/middleware/builtin/llm_response_parser/streaming.go)).
|
||||
([streaming.go:68–73, 144–150](../../../proxy/internal/middleware/builtin/llm_response_parser/streaming.go)).
|
||||
|
||||
3. **`defaults_pricing.yaml` is the source of truth.** Compiled into the
|
||||
binary via `//go:embed`
|
||||
([pricing.go:29–30](../../../../netbird/proxy/internal/llm/pricing/pricing.go)).
|
||||
([pricing.go:29–30](../../../proxy/internal/llm/pricing/pricing.go)).
|
||||
`DefaultTable()` parses once and panics on parse failure
|
||||
([pricing.go:42–49](../../../../netbird/proxy/internal/llm/pricing/pricing.go))
|
||||
([pricing.go:42–49](../../../proxy/internal/llm/pricing/pricing.go))
|
||||
— by design: a broken embedded YAML must not ship to production.
|
||||
|
||||
4. **Loader path validation.** `resolveMiddlewareDataPath`
|
||||
([pricing.go:370–394](../../../../netbird/proxy/internal/llm/pricing/pricing.go))
|
||||
([pricing.go:370–394](../../../proxy/internal/llm/pricing/pricing.go))
|
||||
rejects absolute paths, traversal segments, and basenames that fail
|
||||
`basenameRegex = ^[a-zA-Z0-9._-]+$`. The resolved path must remain
|
||||
inside `baseDir` even after `filepath.Clean`. Tests:
|
||||
@@ -325,19 +316,19 @@ emits `cost.skipped=unknown_model`.
|
||||
5. **Unix loader symlink safety.** `O_NOFOLLOW` on open, `f.Stat()` on the
|
||||
open descriptor (never re-stat by path), `info.Mode().IsRegular()` check,
|
||||
`io.LimitReader(f, maxPricingBytes+1)` with a final size assertion
|
||||
([pricing_unix.go:25–57](../../../../netbird/proxy/internal/llm/pricing/pricing_unix.go)).
|
||||
([pricing_unix.go:25–57](../../../proxy/internal/llm/pricing/pricing_unix.go)).
|
||||
A mid-read symlink swap is detected because the fstat is on the original
|
||||
fd. Test: `TestNewLoader_RejectsOversizedFile_FixesM4`.
|
||||
|
||||
6. **`yaml.NewDecoder(...).KnownFields(true)`**
|
||||
([pricing.go:397–398](../../../../netbird/proxy/internal/llm/pricing/pricing.go))
|
||||
([pricing.go:397–398](../../../proxy/internal/llm/pricing/pricing.go))
|
||||
rejects YAML files that carry fields not in the schema. A typo in an
|
||||
operator override file fails loud instead of silently zeroing rates.
|
||||
|
||||
## Things to scrutinise
|
||||
|
||||
**Correctness.** Verify OpenAI cached-prompt clamp at
|
||||
[pricing.go:147–149](../../../../netbird/proxy/internal/llm/pricing/pricing.go)
|
||||
[pricing.go:147–149](../../../proxy/internal/llm/pricing/pricing.go)
|
||||
short-circuits before subtraction. `Anthropic.TotalTokens` sums all four
|
||||
buckets (in + out + cache_read + cache_creation) — downstream dashboards
|
||||
need to know this differs from `input + output`.
|
||||
@@ -376,7 +367,7 @@ right `cost.skipped` reason.
|
||||
| `sse_test.go` | 12 | Fixture replay both providers; multiline `data:`; CRLF; comment skip; trailing-event-without-blank-line; oversize rejection |
|
||||
| `pricing/pricing_test.go` | 21 | Provider-shape switch; cached-rate fallback; cached-clamp; symlink rejection (target outside basedir + symlink to file); path validation matrix; oversize rejection; reload-keeps-previous-on-parse-error; mtime change detection; goroutine cancellation |
|
||||
|
||||
**Fixtures** ([proxy/internal/llm/fixtures/](../../../../netbird/proxy/internal/llm/fixtures/)):
|
||||
**Fixtures** ([proxy/internal/llm/fixtures/](../../../proxy/internal/llm/fixtures/)):
|
||||
`openai_chat_completion.json` (chat.completions with usage),
|
||||
`openai_responses.json` (Responses API shape),
|
||||
`openai_stream.txt` (3 deltas + usage + `[DONE]`),
|
||||
@@ -396,6 +387,6 @@ content_block_{start,delta×2,stop} → message_delta (usage) → message_stop),
|
||||
- Direct callers: `llm_request_parser/middleware.go:82–94`,
|
||||
`llm_response_parser/middleware.go:113–123`,
|
||||
`llm_response_parser/streaming.go:65, 142`, `cost_meter/factory.go:49–57`.
|
||||
- Out of scope: agent-network synthesiser stamping `provider_id`
|
||||
(`a67022ffa`) — see management-side module review. Proxy server boot +
|
||||
`FactoryContext` construction — see proxy-framework module review.
|
||||
- Related elsewhere: the agent-network synthesiser stamping `provider_id`
|
||||
is covered in the management-side module guide; proxy server boot +
|
||||
`FactoryContext` construction is covered in the proxy-framework guide.
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
# proxy/runtime — translate + serve + log
|
||||
|
||||
> **Reviewer profile:** Proxy maintainer; familiar with the existing `proxy/internal/proxy/reverseproxy.go` request lifecycle, `proxy/internal/accesslog` emission, `proto.ProxyMapping` ingestion via `SyncMappings`/`GetMappingUpdate`, and the `proxy/internal/middleware` framework introduced in modules 30/31/32.
|
||||
> **Time to review:** 75 minutes
|
||||
> **Risk level:** High — every config push from management is translated here, and the chain runs on every HTTP request to a synth target
|
||||
> **Risk level:** High — every config push from management is translated here, and the chain runs on every HTTP request to a synth target.
|
||||
> **Backward-compat impact:** Additive at the wire (`PathTargetOptions.middlewares`, `agent_network`, `disable_access_log`, capture caps) and on the proxy `Server` struct (`MiddlewareDataDir`, `MiddlewareCaptureBudgetBytes`). Non-agent-network targets stay on the no-middleware fast path.
|
||||
|
||||
## Module boundary
|
||||
@@ -11,39 +9,24 @@ Turns the synth-service wire format from `ProxyService.SyncMappings`/`GetMapping
|
||||
|
||||
**Inert for non-agent-network targets**: nil or empty chain → existing fast path (reverseproxy.go:127-139); `SuppressAccessLog` defaults false so the access-log middleware emits unchanged.
|
||||
|
||||
## Commits in scope
|
||||
## Files
|
||||
|
||||
| SHA | Subject | LOC delta (this module) |
|
||||
| --- | ------- | --- |
|
||||
| 3ed29d855 | AN-4b: middleware plumbing in the proxy request path | +711 / −9 |
|
||||
| e64ea4b02 | AN-5: built-in middlewares + registry registration | +16 |
|
||||
| 0f9f56a58 | AN-5b: activate the middleware system in the proxy server | +136 / −1 |
|
||||
| 9a1547143 | AN-6: access-log `agent_network` flag end-to-end | +3 |
|
||||
| 9ebe219fd | test: lock the auth→middleware group-propagation wiring | +136 |
|
||||
| 468875cb4 | Wire `EnableLogCollection` to suppress access-log | +222 / −5 |
|
||||
| b438a7194 | Carry identity onto respInput (PII redaction commit; this module: respInput fix only) | +12 / −4 |
|
||||
| 2a0d4991b | Self-contained full-chain integration test | +314 |
|
||||
|
||||
Total in scope: ~1545 added / 14 deleted across 14 files.
|
||||
|
||||
## Files changed
|
||||
|
||||
| Path | LOC | Role |
|
||||
| ---- | --- | ---- |
|
||||
| proxy/middleware_translate.go | +165 | proto→Spec translation; slot/failmode/timeout mapping; caps |
|
||||
| proxy/middleware_translate_test.go | +246 | translator unit tests |
|
||||
| proxy/middleware_register.go | +16 | blank-imports the eight builtins for `init()` registration |
|
||||
| proxy/server.go | +129 | `initMiddlewareManager`, `rebuildMiddlewareChains`, `isLiveService`, `buildMiddlewareBindings`, new Server fields, `protoToMapping` stamps AgentNetwork/DisableAccessLog/CaptureConfig/Middlewares |
|
||||
| proxy/internal/proxy/reverseproxy.go | +289 / −9 | `WithMiddlewareManager`, chain dispatch, body capture, `applyUpstreamRewrite`/`Headers`, `buildRequestInput`, response-leg respInput identity fields |
|
||||
| proxy/internal/proxy/reverseproxy_test.go | +44 | `TestBuildRequestInput_PropagatesIdentityAndGroups` |
|
||||
| proxy/internal/proxy/context.go | +43 | `agentNetwork`, `suppressAccessLog`, `userGroupNames` on `CapturedData` |
|
||||
| proxy/internal/proxy/servicemapping.go | +16 | new `PathTarget` fields |
|
||||
| proxy/internal/proxy/agent_network_chain_realstack_test.go | +314 | end-to-end self-contained chain test |
|
||||
| proxy/internal/accesslog/logger.go | +2 | `logEntry.AgentNetwork` → `proto.AccessLog` |
|
||||
| proxy/internal/accesslog/middleware.go | +9 / −1 | reads `GetAgentNetwork()`; gates `l.log` on `!GetSuppressAccessLog()` |
|
||||
| proxy/internal/accesslog/middleware_test.go | +185 | suppress/default/preserves-usage assertions |
|
||||
| proxy/internal/auth/middleware_test.go | +92 | tunnel-peer group propagation contract |
|
||||
| proxy/internal/metrics/metrics.go | +9 | `Meter()` getter for the middleware manager |
|
||||
| Path | Role |
|
||||
| ---- | ---- |
|
||||
| proxy/middleware_translate.go | proto→Spec translation; slot/failmode/timeout mapping; caps |
|
||||
| proxy/middleware_translate_test.go | translator unit tests |
|
||||
| proxy/middleware_register.go | blank-imports the eight builtins for `init()` registration |
|
||||
| proxy/server.go | `initMiddlewareManager`, `rebuildMiddlewareChains`, `isLiveService`, `buildMiddlewareBindings`, new Server fields, `protoToMapping` stamps AgentNetwork/DisableAccessLog/CaptureConfig/Middlewares |
|
||||
| proxy/internal/proxy/reverseproxy.go | `WithMiddlewareManager`, chain dispatch, body capture, `applyUpstreamRewrite`/`Headers`, `buildRequestInput`, response-leg respInput identity fields |
|
||||
| proxy/internal/proxy/reverseproxy_test.go | `TestBuildRequestInput_PropagatesIdentityAndGroups` |
|
||||
| proxy/internal/proxy/context.go | `agentNetwork`, `suppressAccessLog`, `userGroupNames` on `CapturedData` |
|
||||
| proxy/internal/proxy/servicemapping.go | new `PathTarget` fields |
|
||||
| proxy/internal/proxy/agent_network_chain_realstack_test.go | end-to-end self-contained chain test |
|
||||
| proxy/internal/accesslog/logger.go | `logEntry.AgentNetwork` → `proto.AccessLog` |
|
||||
| proxy/internal/accesslog/middleware.go | reads `GetAgentNetwork()`; gates `l.log` on `!GetSuppressAccessLog()` |
|
||||
| proxy/internal/accesslog/middleware_test.go | suppress/default/preserves-usage assertions |
|
||||
| proxy/internal/auth/middleware_test.go | tunnel-peer group propagation contract |
|
||||
| proxy/internal/metrics/metrics.go | `Meter()` getter for the middleware manager |
|
||||
|
||||
## Architecture & flow
|
||||
|
||||
@@ -99,7 +82,7 @@ sequenceDiagram
|
||||
RP->>RP: capturingWriter + applyUpstreamRewrite/Headers
|
||||
RP->>U: httputil.ReverseProxy.ServeHTTP(respWriter)
|
||||
U-->>RP: response
|
||||
RP->>CH: RunResponse (respInput carries UserGroups, b438a7194)
|
||||
RP->>CH: RunResponse (respInput carries UserGroups)
|
||||
RP->>CH: RunTerminal (merged request+response metadata)
|
||||
end
|
||||
end
|
||||
@@ -141,10 +124,10 @@ At **request time** the access-log middleware stamps `CapturedData`; the auth ch
|
||||
|
||||
## Invariants
|
||||
|
||||
- **Synth-service updates are live (no proxy restart).** Every `MODIFIED` flows through `modifyMapping → cleanupMappingRoutes` (invalidates chains) `→ setupHTTPMapping → updateMapping → rebuildMiddlewareChains`. **Note 263dabd73 (ProxyMapping.Private preservation):** the bug it fixed lives in `management/internals/shared/grpc/proxy.go:shallowCloneMapping`, not this module, but it surfaces here — without the fix, every `MODIFIED` synth service arrived `private=false`, auth skipped `ValidateTunnelPeer`, `CapturedData.UserGroups` stayed empty, `llm_router` denied with `llm_policy.no_authorised_provider`, and only a management restart worked around it. Review this module assuming `mapping.GetPrivate()` is correct on every batch — that assumption is what the cited fix established.
|
||||
- **Synth-service updates are live (no proxy restart).** Every `MODIFIED` flows through `modifyMapping → cleanupMappingRoutes` (invalidates chains) `→ setupHTTPMapping → updateMapping → rebuildMiddlewareChains`. **ProxyMapping.Private preservation:** the relevant logic lives in `management/internals/shared/grpc/proxy.go:shallowCloneMapping`, not this module, but it surfaces here — if a `MODIFIED` synth service arrives `private=false`, auth skips `ValidateTunnelPeer`, `CapturedData.UserGroups` stays empty, and `llm_router` denies with `llm_policy.no_authorised_provider` until a management restart re-pushes the snapshot. This module assumes `mapping.GetPrivate()` is correct on every batch.
|
||||
- **`EnableLogCollection=false` suppresses access-log writes but middleware still runs.** Gate is one `if !cd.GetSuppressAccessLog()` immediately around `l.log(entry)` (middleware.go:95); `trackUsage` runs below the gate. Locked by `TestMiddleware_SuppressAccessLog_PreservesUsageTracking` (middleware_test.go:139).
|
||||
- **`agent_network` flag on access-log entries is set when the chain processed the request.** Source `target.AgentNetwork`, stamped at reverseproxy.go:105, read at accesslog/middleware.go:86.
|
||||
- **auth → builtin group propagation (9ebe219fd locked this).** `Protect` writes `UserGroups`/`UserGroupNames`; `buildRequestInput` (reverseproxy.go:333) copies them into `middleware.Input`. After b438a7194 the response-leg `respInput` (reverseproxy.go:196-223) also carries `UserEmail`/`UserGroups`/`UserGroupNames` — `llm_limit_record` needs `UserGroups` to ship `group_ids` so management's group-targeted budget rules match (comment at reverseproxy.go:211-215).
|
||||
- **auth → builtin group propagation.** `Protect` writes `UserGroups`/`UserGroupNames`; `buildRequestInput` (reverseproxy.go:333) copies them into `middleware.Input`. The response-leg `respInput` (reverseproxy.go:196-223) also carries `UserEmail`/`UserGroups`/`UserGroupNames` — `llm_limit_record` needs `UserGroups` to ship `group_ids` so management's group-targeted budget rules match (comment at reverseproxy.go:211-215).
|
||||
- **Empty chains stay on the fast path.** `ServeHTTP` skips body capture and the run sequence when `chain == nil || chain.Empty()` (reverseproxy.go:127).
|
||||
- **Self-registration is the only way a builtin reaches the registry.** `middleware_register.go` blank-imports each builtin; `init()` adds the factory to `mwbuiltin.DefaultRegistry()`. Missing it → translator drops the entry with a warn (translate.go:97).
|
||||
|
||||
@@ -153,7 +136,7 @@ At **request time** the access-log middleware stamps `CapturedData`; the auth ch
|
||||
### Correctness
|
||||
- **Translate edge cases** — drops on nil cfg, empty ID, unknown ID, UNSPECIFIED slot; each logs one warn; volume bounded by `MaxMiddlewaresPerChain`.
|
||||
- **Re-translate without dropping in-flight requests** — `Manager.Rebuild` is the only call from `rebuildMiddlewareChains`. Reverse proxy reads `ChainFor` once per request (reverseproxy.go:327) and runs the captured `*Chain` for the whole request. Verify in module 30 that `Rebuild` swaps atomically.
|
||||
- **ProxyMapping.Private preservation** — fixed in 263dabd73 (management). Proxy-side regression catches: `TestProtect_PrivateService_TunnelPeerGroupsPropagate` + the integration test.
|
||||
- **ProxyMapping.Private preservation** — enforced management-side in `shallowCloneMapping`. Proxy-side regression catches: `TestProtect_PrivateService_TunnelPeerGroupsPropagate` + the integration test.
|
||||
- **Body-capture cleanup** — `defer releaseBudget()` (reverseproxy.go:145) and `defer capturingWriter.Release()` (reverseproxy.go:180) must run on every return; confirm no future `return` lands between acquisition and defer.
|
||||
- **`applyUpstreamRewrite` clones the URL** — `cloned := *orig` value-copies `*url.URL`; safe because overwritten fields are strings, not slices/maps (reverseproxy.go:285-292).
|
||||
|
||||
@@ -190,11 +173,11 @@ At **request time** the access-log middleware stamps `CapturedData`; the auth ch
|
||||
| --------- | ---------- |
|
||||
| proxy/middleware_translate_test.go | Empty/nil → nil; field preservation; unknown ID skip; nil registry permissive; timeout clamping; fail-mode + slot incl. UNSPECIFIED-drop; empty-ID drop; truncation above + at `MaxMiddlewaresPerChain` |
|
||||
| proxy/internal/proxy/reverseproxy_test.go | Rewrite host/headers/cookies/query; trusted proxy; path forwarding; classifyProxyError; X-NetBird-User/Groups anti-spoof + CSV-join + control-char/comma rejection + fallback-to-ID; `TestBuildRequestInput_PropagatesIdentityAndGroups` (UserGroups/Email/GroupNames/AgentNetwork reach `middleware.Input`) |
|
||||
| proxy/internal/proxy/agent_network_chain_realstack_test.go | **THE end-to-end integration test added by 2a0d4991b.** Drives a real agent-network request through `ReverseProxy.ServeHTTP` with the chain the synthesizer produces, against an in-process management gRPC (bufconn) backed by a real sqlite store + real `agentnetwork.Manager`, plus an `httptest` upstream. No tilt/docker/real LLM. Guarantees: (1) response-leg `respInput` carries `UserGroups` so `llm_limit_record` ships non-empty `group_ids` and the admin-group consumption row increments — the exact regression class b438a7194 fixed; (2) `RedactPii=true` redacts both prompt and completion on captured metadata; (3) full chain runs against a real management stack. **Line 189-211 inlines the proto→Spec mapping** instead of calling the proxy's private `translateMiddlewareConfig` — keep that inline mirror in sync with `proxy/middleware_translate.go` or the test silently diverges from production. |
|
||||
| proxy/internal/proxy/agent_network_chain_realstack_test.go | **The end-to-end integration test.** Drives a real agent-network request through `ReverseProxy.ServeHTTP` with the chain the synthesizer produces, against an in-process management gRPC (bufconn) backed by a real sqlite store + real `agentnetwork.Manager`, plus an `httptest` upstream — no external infrastructure or real LLM. Guarantees: (1) response-leg `respInput` carries `UserGroups` so `llm_limit_record` ships non-empty `group_ids` and the admin-group consumption row increments; (2) `RedactPii=true` redacts both prompt and completion on captured metadata; (3) the full chain runs against a real management stack. **Line 189-211 inlines the proto→Spec mapping** instead of calling the proxy's private `translateMiddlewareConfig` — keep that inline mirror in sync with `proxy/middleware_translate.go` or the test silently diverges from production. |
|
||||
| proxy/internal/accesslog/middleware_test.go | `SuppressAccessLog=true` skips `SendAccessLog` (150ms negative wait); default emits one send (2s positive); usage tracking runs under suppression |
|
||||
| proxy/internal/auth/middleware_test.go | `TestProtect_PrivateService_TunnelPeerGroupsPropagate` proves `peer_group_ids` reach `CapturedData.UserGroups`; `TestProtect_PrivateService_TunnelPeerDenied` proves rejected peers 403 without reaching the handler |
|
||||
|
||||
The integration test replaces the bash 50/51 e2e legs in ~5s, no docker/tilt — exercising the real synthesizer, `Manager.Rebuild`, `ServeHTTP` dispatch, and `llm_limit_record` writing a real consumption row through the real `agentnetwork.Manager` over real gRPC.
|
||||
The integration test runs in a few seconds with no external infrastructure — exercising the real synthesizer, `Manager.Rebuild`, `ServeHTTP` dispatch, and `llm_limit_record` writing a real consumption row through the real `agentnetwork.Manager` over real gRPC.
|
||||
|
||||
## Known limitations / explicit non-goals
|
||||
|
||||
@@ -202,7 +185,7 @@ The integration test replaces the bash 50/51 e2e legs in ~5s, no docker/tilt —
|
||||
- **No throttle on management push rate** — every `MODIFIED` triggers `Manager.Rebuild`. Mitigation upstream.
|
||||
- **Streaming responses (SSE)** — body capture is streaming-aware, but response-leg middleware runs only after the response completes; long SSE streams delay `llm_limit_record` until close.
|
||||
- **OIDC-only path doesn't carry tunnel-peer groups** — agent-network synth services rely on the Private tunnel-peer path; JWT groups claim is the only carrier for non-Private OIDC.
|
||||
- **`agent_network` flag on L4 entries** not added; HTTP-only in this PR.
|
||||
- **`agent_network` flag on L4 entries** not added; HTTP-only.
|
||||
- **`mw.capture.bypass_reason` metadata key** documented at reverseproxy.go:151,184; namespace this in module 30/31 to avoid collisions.
|
||||
|
||||
## Cross-references
|
||||
|
||||
@@ -1,38 +1,32 @@
|
||||
# dashboard — UI for agent-networks
|
||||
|
||||
> **Reviewer profile:** Dashboard maintainer; familiar with the Next.js app-router under `src/app/(dashboard)`, the existing Tabs / Modal / DataTable components, the Providers / Permissions / Groups context pattern, and the React-Flow control-center graph. Light familiarity with the agent-network REST routes is enough.
|
||||
> **Time to review:** 90 minutes (broad surface; many small components, a few large ones)
|
||||
> **Risk level:** Medium. New surface is isolated under `src/modules/agent-network/` and `src/app/(dashboard)/agent-network/`, but the PR also reshapes the sidebar, splits `/peers`, renames `reverse-proxy/clusters` → `self-hosted-proxies`, and overlays the Control Center graph. Regressions here would be cross-cutting.
|
||||
> **Backward-compat impact:** Additive on the API side. Breaking on URL/navigation: `/peers` redirects to `/peers/devices` (src/app/(dashboard)/peers/page.tsx:7-15), `/reverse-proxy/clusters` was renamed to `/reverse-proxy/self-hosted-proxies`, the sidebar lost Access Control / Networks / Reverse Proxy / DNS / standalone Guardrails / Consumption / Activity (Navigation.tsx:165-171 — routes still resolve via URL), and the standalone `/agent-network/{access-log,consumption,global-controls}` routes are gone in favor of `/agent-network/observability` (3fbe44e).
|
||||
This module documents code that lives in the **dashboard repo** (under
|
||||
`src/modules/agent-network/` and `src/app/(dashboard)/agent-network/`), not
|
||||
in this repo. It is co-located here so backend readers see the full picture.
|
||||
|
||||
> **Risk level:** Medium. The new surface is isolated under `src/modules/agent-network/` and `src/app/(dashboard)/agent-network/`, but it also reshapes the sidebar, splits `/peers`, renames `reverse-proxy/clusters` → `self-hosted-proxies`, and overlays the Control Center graph. Regressions here would be cross-cutting.
|
||||
> **Backward-compat impact:** Additive on the API side. Breaking on URL/navigation: `/peers` redirects to `/peers/devices` (src/app/(dashboard)/peers/page.tsx:7-15), `/reverse-proxy/clusters` was renamed to `/reverse-proxy/self-hosted-proxies`, the sidebar lost Access Control / Networks / Reverse Proxy / DNS / standalone Guardrails / Consumption / Activity (Navigation.tsx:165-171 — routes still resolve via URL), and the standalone `/agent-network/{access-log,consumption,global-controls}` routes are gone in favor of `/agent-network/observability`.
|
||||
|
||||
## Module boundary
|
||||
|
||||
The dashboard is the only place an operator interacts with agent-networks: provider catalog, configured providers, policies, guardrails, account-level budget rules, account settings (collection / redaction toggles), per-request access log, and consumption rollups all render, paginate, and edit here. Data flows in via SWR (`useFetchApi`) keyed by REST URL. One big context provider (`src/modules/agent-network/AIProvidersProvider.tsx`) aggregates five resources (providers, policies, guardrails, budget rules, settings) plus the proxy access-log stream filtered to `agent_network=true`, and exposes `add* / update* / toggle* / delete*` mutators that call through `useApiCall` and re-`mutate()` SWR. Pages mount the provider once at the top and compose presentational tables and modals beneath. The control-center page additionally fetches `/agent-network/{providers,policies}` directly (control-center/page.tsx:123-130) to overlay graph nodes.
|
||||
|
||||
## Commits in scope (newest first)
|
||||
## What the UI delivers
|
||||
|
||||
| SHA | Subject | LOC delta |
|
||||
| --- | ------- | --------- |
|
||||
| 3fbe44e | [agent-network] Merge Global Controls + Access Log into AI Observability | +160 / −207 |
|
||||
| 1988e92 | Global Controls page (account toggles + budget rules) | +1453 / −1 |
|
||||
| 95b4daa | control center: open agent-policy modal on node click; hide rows-per-page | +51 / −3 |
|
||||
| b0c4f3e | peers table: compact toolbar + user filter | (peers + UserFilterSelector) |
|
||||
| a61581b | control center + peers: agent-network overlay + width trims | (control-center + peers) |
|
||||
| e159d6b / d2ea1fb | control center: drop All Networks dropdown; hide Networks tab | (FlowSelector + page) |
|
||||
| 7bf59b8 | rename peer sub-views to User Devices and Agents & Servers | (peers + PeersListView) |
|
||||
| a65a956 | repackage dashboard nav for agent-network focus | (Navigation.tsx) |
|
||||
| e7bdc5e / 672195a | reverse-proxy: Private toggle + cluster target; drop body-capture knobs | (reverse-proxy) |
|
||||
| f0b5214 / 4be7d3c / 2875ade / eb54136 / 924537f / c446eec / e042ca3 | Provider modal: OpenRouter, Vercel, Cloudflare, Bifrost (×2), Portkey, LiteLLM Mappings | (AIProviderModal) |
|
||||
| 1f24392 | window_hours → window_seconds; minute picker on Limits | (PolicyLimitsTab) |
|
||||
| 2157fec / 59564fd | Consumption page: charts + filters; basic counter view | (ConsumptionPanel + Table) |
|
||||
| 1af6754 | resolve providers by config-row ID in access log | (AccessLogTable) |
|
||||
| 6b91b7a | account-level endpoint surface on providers page | (providers/page.tsx) |
|
||||
| 51221e8 / 6a26ccb | wire access log to /events/proxy?agent_network=true; resolve group IDs | (provider context) |
|
||||
| b50a689 | Drop Budgets & Alerts; rebuild Access Log in Proxy Events style | (rewrite) |
|
||||
| 2b7f746 / 5e10a2b / 4d3ac8d / 77a7070 | Wire guardrails / policies / providers / catalog to backend | (provider context) |
|
||||
| 4cf718f | Add Agent Network dashboard preview | +6036 LOC (initial scaffold) |
|
||||
|
||||
Total branch delta vs `main`: ~ +10.9k / −890 LOC across 82 files.
|
||||
- **AI Observability** page with four tabs: Access Logs, Budget Dashboard,
|
||||
Budget Settings, Log Settings (replaces the standalone access-log,
|
||||
consumption, and global-controls routes).
|
||||
- **Providers** page: provider catalog + connect/edit wizard with per-vendor
|
||||
copy (LiteLLM, Portkey, Bifrost, Cloudflare, Vercel, OpenRouter, custom).
|
||||
- **Policies** page: group → provider authorization with per-policy Limits
|
||||
(minute-granular windows) + guardrail attach.
|
||||
- **Guardrails** page: reusable model-allowlist + prompt-capture sets.
|
||||
- **Account controls**: Log Collection / Prompt Collection / Redact PII toggles.
|
||||
- **Budget rules**: account-level rules reusing the policy Limits UI.
|
||||
- **Control Center overlay**: provider + agent-policy nodes on the graph.
|
||||
- **Navigation + peers reshaping**: peers split into Devices / Agents,
|
||||
`reverse-proxy/clusters` renamed to `self-hosted-proxies`, sidebar
|
||||
repackaged for agent-network focus.
|
||||
|
||||
## Surface added
|
||||
|
||||
@@ -44,11 +38,11 @@ Total branch delta vs `main`: ~ +10.9k / −890 LOC across 82 files.
|
||||
| `/agent-network/providers` | List + connect providers; header surfaces per-account base URL | providers/page.tsx + AgentProvidersTable + AIProviderModal |
|
||||
| `/agent-network/policies` | Group → Provider authorization with per-policy Limits + Guardrail attach | policies/page.tsx + AgentPoliciesTable + AgentPolicyModal |
|
||||
| `/agent-network/guardrails` | Reusable guardrail sets (model allowlist + prompt capture) | guardrails/page.tsx + AgentGuardrailsTable + AgentGuardrailModal |
|
||||
| `/agent-network/observability` | NEW (3fbe44e). Tabs: Access Logs / Budget Dashboard / Budget Settings / Log Settings | observability/page.tsx |
|
||||
| `/agent-network/observability` | Tabs: Access Logs / Budget Dashboard / Budget Settings / Log Settings | observability/page.tsx |
|
||||
| `/peers/devices`, `/peers/agents` | Split of `/peers`, shared via `PeersListView` keyed by `kind` | peers/{devices,agents}/page.tsx |
|
||||
| `/reverse-proxy/self-hosted-proxies` | Renamed from `clusters` | self-hosted-proxies/page.tsx |
|
||||
|
||||
Deleted in 3fbe44e: `/agent-network/access-log`, `/agent-network/consumption`, `/agent-network/global-controls`.
|
||||
Removed in favor of `/agent-network/observability`: `/agent-network/access-log`, `/agent-network/consumption`, `/agent-network/global-controls`.
|
||||
|
||||
### New modules under src/modules/agent-network
|
||||
|
||||
@@ -68,9 +62,9 @@ Deleted in 3fbe44e: `/agent-network/access-log`, `/agent-network/consumption`, `
|
||||
|
||||
### Touched non-agent-network areas
|
||||
|
||||
- **control-center**: agent-network overlay (provider + agent-policy nodes); removed All Networks dropdown (a61581b, e159d6b); hid Networks tab in FlowSelector (FlowSelector.tsx:9-14 — enum value kept so `?tab=networks` still type-checks); wrapped `ControlCenterView` in `AIProvidersProvider` (page.tsx:73-83); `agentPolicyNode` clicks routed to a separate state slot (page.tsx:1871-1874). New node renderers: nodes/ProviderNode.tsx, nodes/AgentPolicyNode.tsx (registered at utils/nodes.ts:21-22).
|
||||
- **control-center**: agent-network overlay (provider + agent-policy nodes); removed the All Networks dropdown; hid the Networks tab in FlowSelector (FlowSelector.tsx:9-14 — enum value kept so `?tab=networks` still type-checks); wrapped `ControlCenterView` in `AIProvidersProvider` (page.tsx:73-83); `agentPolicyNode` clicks routed to a separate state slot (page.tsx:1871-1874). New node renderers: nodes/ProviderNode.tsx, nodes/AgentPolicyNode.tsx (registered at utils/nodes.ts:21-22).
|
||||
- **peers**: Split into Devices and Agents sub-routes; shared via `PeersListView` keyed by `kind` (PeersListView.tsx:24-95). New compact-toolbar `UserFilterSelector` (users/UserFilterSelector.tsx).
|
||||
- **reverse-proxy**: Folder rename `clusters/` → `self-hosted-proxies/`; deleted `ClustersFeaturesCell.tsx`, `ClusterTypeIndicator.tsx`; new ReverseProxyClusterTargetSelector for cluster target type (e7bdc5e); Private toggle on target modal; body-capture knobs removed (672195a); new ReverseProxyEventExpandedRow.
|
||||
- **reverse-proxy**: Folder rename `clusters/` → `self-hosted-proxies/`; deleted `ClustersFeaturesCell.tsx`, `ClusterTypeIndicator.tsx`; new ReverseProxyClusterTargetSelector for cluster target type; Private toggle on target modal; body-capture knobs removed; new ReverseProxyEventExpandedRow.
|
||||
- **events**: `ReverseProxyEventsUserCell` rewritten with user + peer fallback (ReverseProxyEventsUserCell.tsx:14-21), shared with the access-log table.
|
||||
- **navigation**: Full repackaging in Navigation.tsx — Agent Network items flattened (no collapsible parent), distinct icons per item; Access Control, Networks, Reverse Proxy, DNS, standalone Guardrails, Consumption, Activity removed (still URL-reachable, per lines 165-171).
|
||||
|
||||
@@ -84,7 +78,7 @@ graph TD
|
||||
Nav --> ProvidersPage[/agent-network/providers/]
|
||||
Nav --> PoliciesPage[/agent-network/policies/]
|
||||
Nav --> GuardrailsPage[/agent-network/guardrails/]
|
||||
Nav --> ObsPage[/agent-network/observability NEW/]
|
||||
Nav --> ObsPage[/agent-network/observability/]
|
||||
|
||||
ProvidersPage --> AIPP1[AIProvidersProvider]
|
||||
PoliciesPage --> AIPP2[AIProvidersProvider]
|
||||
@@ -112,7 +106,7 @@ graph TD
|
||||
BudgetModal -.reuses.-> PolLimitsTab
|
||||
```
|
||||
|
||||
### AI Observability tab page (today's commit)
|
||||
### AI Observability tab page
|
||||
|
||||
```mermaid
|
||||
graph LR
|
||||
@@ -178,9 +172,9 @@ Every list view reaches management through SWR over `/api/agent-network/*`. The
|
||||
|
||||
- **Tab-state URL hand-off is one-way.** observability/page.tsx:53-58 reads `?tab=` on mount (despite the file comment at line 28 saying URL hand-off is future) but `setTab` does NOT push back, so reload preserves the chosen tab only if it came in via the link. Inconsistent with control-center (page.tsx:1817-1831).
|
||||
- **Provider overlay runs only in `applySingleGroupView` / `applyPeerView`** (control-center/page.tsx:557, 1159-1166). User view does NOT show providers — if agent-network is a primary lens, that's a gap.
|
||||
- **Two useEffects race to invalidate the control-center layout.** page.tsx:1655-1657 drops `layoutInitialized` when `agentPolicies` / `agentProviders` arrive; the main effect (1786-1799) also lists them as deps. Works today but fragile — watch for flash-of-empty-graph.
|
||||
- **Two useEffects race to invalidate the control-center layout.** page.tsx:1655-1657 drops `layoutInitialized` when `agentPolicies` / `agentProviders` arrive; the main effect (1786-1799) also lists them as deps. Functional but fragile — watch for flash-of-empty-graph.
|
||||
- **`updateProvider` / `updatePolicy` / `updateBudgetRule` use `??` on `enabled`** (AIProvidersProvider.tsx:784, 859, 1018). Toggle paths are safe; any caller sending `enabled: false` thinking "leave it off" gets `existing.enabled` instead. Audit modal callers.
|
||||
- **Form validation in modals is minimal.** Window-seconds picker (1f24392) — mockData.ts:209-215 documents "minimum 60 — one minute" but I couldn't find a matching UI guard in PolicyLimitsTab.
|
||||
- **Form validation in modals is minimal.** Window-seconds picker — mockData.ts:209-215 documents "minimum 60 — one minute" but there is no matching UI guard in PolicyLimitsTab; the backend validator is the enforcement point.
|
||||
|
||||
### Security
|
||||
|
||||
@@ -203,7 +197,7 @@ Every list view reaches management through SWR over `/api/agent-network/*`. The
|
||||
|
||||
### Visual consistency
|
||||
|
||||
- Observability tab style claims to match peer/page.tsx (3fbe44e commit msg). Outer Tabs `pt-4 pb-0 mb-0`, TabsList `px-8` (observability/page.tsx:94-96) — confirm chrome height matches so the page doesn't visually jump.
|
||||
- The observability tab style mirrors peers/page.tsx. Outer Tabs `pt-4 pb-0 mb-0`, TabsList `px-8` (observability/page.tsx:94-96) — confirm chrome height matches so the page doesn't visually jump.
|
||||
- Sidebar: `Boxes` for Providers, `AccessControlIcon` for Policies, `TelescopeIcon` for AI Observability (Navigation.tsx:113,120,133). Reusing `AccessControlIcon` makes Policies look identical to the (now hidden) Access Control item — if Access Control ever comes back, they collide.
|
||||
- `AgentNetworkIcon` is used in breadcrumbs on every agent-network page but NOT in the sidebar (per-page icons instead). Deliberate departure — record so it doesn't get reverted.
|
||||
|
||||
@@ -212,7 +206,7 @@ Every list view reaches management through SWR over `/api/agent-network/*`. The
|
||||
- **Cypress**: One file (`cypress/e2e/test.cy.ts`) covering only the install-page copy-to-clipboard flow. NOTHING covers agent-network UI.
|
||||
- **Component / unit tests**: `src/utils/version.test.ts` is the only `.test.*` file in the repo. The agent-network modules ship without component tests.
|
||||
- Data-cy hooks exist on key controls: `save-account-controls` (AgentAccountControlsCard.tsx:71), `enable-log-collection`, `enable-prompt-collection`, `redact-pii`, plus existing `data-cy={policy.name}` / `data-cy={provider.name}` on ActiveInactiveRow. Sufficient hooks for Cypress flows; none written yet.
|
||||
- **Tooling gap (not introduced by this PR):** `npm run lint` (`next lint`) is broken in Next 16 — the `lint` subcommand was removed from the Next CLI in 16.x. Same script lives on `main`, but the merge means the dashboard effectively ships without a working lint gate. Worth flagging now; add either a flat-config `eslint .` script or wire ESLint via an explicit `eslint-config-next` invocation.
|
||||
- **Tooling gap (pre-existing):** `npm run lint` (`next lint`) is broken in Next 16 — the `lint` subcommand was removed from the Next CLI in 16.x, so the dashboard effectively has no working lint gate. The fix is to add either a flat-config `eslint .` script or wire ESLint via an explicit `eslint-config-next` invocation.
|
||||
|
||||
## Known limitations / explicit non-goals
|
||||
|
||||
@@ -223,12 +217,12 @@ Every list view reaches management through SWR over `/api/agent-network/*`. The
|
||||
- **`FlowView.NETWORKS` retained but hidden** from FlowSelector (FlowSelector.tsx:9-14). Old `?tab=networks` links still route to the hidden view because `applyNetworksView` still runs.
|
||||
- **Redirects are not query-preserving** — `router.replace("/peers/devices")` (peers/page.tsx:13) strips any incoming filter params.
|
||||
- **Control-center cross-fetches** `/agent-network/{providers,policies}` directly on top of `AIProvidersProvider`. Could be collapsed.
|
||||
- **Sidebar permanently hides Access Control, Networks, Reverse Proxy, standalone Guardrails, DNS, Activity, Consumption.** Routes still resolve via URL (Navigation.tsx:165-171); intentional per a65a956.
|
||||
- **Sidebar permanently hides Access Control, Networks, Reverse Proxy, standalone Guardrails, DNS, Activity, Consumption.** Routes still resolve via URL (Navigation.tsx:165-171); intentional.
|
||||
|
||||
## Cross-references
|
||||
|
||||
- Upstream API contracts: [shared/api](10-shared-api.md)
|
||||
- Backend persistence: [management/store](20-management-store.md)
|
||||
- Backend handler wiring: [management/handlers](22-management-handlers-wiring.md) (planned)
|
||||
- Backend handler wiring: [management/handlers + wiring](22-management-handlers-wiring.md)
|
||||
- End-to-end flow narrative: [../01-end-to-end-flows.md](../01-end-to-end-flows.md)
|
||||
- Top-level overview: [../00-overview.md](../00-overview.md)
|
||||
|
||||
@@ -1,13 +1,13 @@
|
||||
# path-routed providers — Vertex AI + Bedrock
|
||||
|
||||
> Reviewer profile: proxy / LLM + management catalog. You should be comfortable
|
||||
> with the `llm_router` / `llm_request_parser` middlewares
|
||||
> ([31-proxy-middleware-builtin.md](31-proxy-middleware-builtin.md)), the
|
||||
> per-provider parser surface ([32-proxy-llm-parsers.md](32-proxy-llm-parsers.md)),
|
||||
> and the synthesiser's catalog → `ProviderRoute` mapping
|
||||
> ([21-management-agentnetwork.md](21-management-agentnetwork.md)). This guide
|
||||
> pulls the **path-routed** provider story together in one place because it
|
||||
> crosses the catalog, the synthesiser, the request parser, and the router.
|
||||
This guide pulls the **path-routed** provider story together in one place
|
||||
because it crosses the catalog, the synthesiser, the request parser, and the
|
||||
router. The relevant building blocks are the `llm_router` /
|
||||
`llm_request_parser` middlewares
|
||||
([31-proxy-middleware-builtin.md](31-proxy-middleware-builtin.md)), the
|
||||
per-provider parser surface ([32-proxy-llm-parsers.md](32-proxy-llm-parsers.md)),
|
||||
and the synthesiser's catalog → `ProviderRoute` mapping
|
||||
([21-management-agentnetwork.md](21-management-agentnetwork.md)).
|
||||
|
||||
Sibling modules: [31-proxy-middleware-builtin.md](31-proxy-middleware-builtin.md)
|
||||
(router + request parser) and [32-proxy-llm-parsers.md](32-proxy-llm-parsers.md)
|
||||
|
||||
Reference in New Issue
Block a user