Skip to content

Commit 79445a6

Browse files
authored
Merge pull request #101 from linuxfoundation/lfxv2-2468-agent-docs-review
docs: agent documentation review pass (LFXV2-2468)
2 parents 5845a53 + 42f6a8e commit 79445a6

9 files changed

Lines changed: 368 additions & 438 deletions

File tree

.agents/skills/upgrade-maintenance/SKILL.md

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ description: Upgrade all Go dependencies, fix payload/contract changes introduce
44
license: MIT
55
---
66

7+
# Upgrade Maintenance
8+
79
Perform a full upgrade maintenance pass on the lfx-mcp codebase. This covers
810
three sequential phases: dependency upgrades, upstream contract fixes, and a
911
query service argument review.
@@ -94,7 +96,7 @@ version that is a subdirectory **inside** the `go.opentelemetry.io/otel` module.
9496
the semconv sub-package they were written against. When that version differs from
9597
the one imported in our own code, the OTel SDK emits a startup error:
9698

97-
```
99+
```text
98100
conflicting Schema URL: https://opentelemetry.io/schemas/1.41.0 and
99101
https://opentelemetry.io/schemas/1.40.0
100102
```
@@ -285,11 +287,11 @@ value. Contracts live at:
285287

286288
The filter-to-mechanism mapping for query service payloads:
287289

288-
| Mechanism | Query service field | Index field |
289-
|---|---|---|
290-
| `payload.Parent = "<type>:<uid>"` | `Parent` | `parent_refs` |
291-
| `payload.Tags = ["<key>:<value>"]` | `Tags` | `tags` |
292-
| `payload.Filters = ["<field>:<value>"]` | `Filters` | top-level doc fields |
290+
| Mechanism | Query service field | Index field |
291+
|-----------------------------------------|---------------------|----------------------|
292+
| `payload.Parent = "<type>:<uid>"` | `Parent` | `parent_refs` |
293+
| `payload.Tags = ["<key>:<value>"]` | `Tags` | `tags` |
294+
| `payload.Filters = ["<field>:<value>"]` | `Filters` | top-level doc fields |
293295

294296
### Step 2.3 — Verify
295297

.agents/skills/validate-search-filters/SKILL.md

Lines changed: 46 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ license: MIT
55
compatibility: Requires kubectl configured against the LFX v2 Kubernetes cluster (dev or prod). The OpenSearch cluster is an AWS-managed OpenSearch Service domain reachable only from within the cluster network — queries are tunnelled through the NATS box pod using kubectl exec.
66
---
77

8+
# Validate Search Filters
9+
810
Validate every filter parameter across all tools that use the query service SDK
911
in `internal/tools/` against the live OpenSearch `resources` index and the
1012
upstream indexer-contract documentation. Produce a per-filter verdict table
@@ -79,14 +81,14 @@ grep -rEn "QueryResources|QueryResourcesPayload" internal/tools/ | grep -v "_tes
7981
For each file that appears, read the handler and record how each filter
8082
parameter is sent to the query service. The mechanisms are:
8183

82-
| Mechanism | Query service field | Index field |
83-
|---|---|---|
84-
| `payload.Parent = "<type>:<uid>"` | `Parent` | `parent_refs` |
85-
| `payload.Tags = ["<key>:<value>"]` | `Tags` | `tags` |
86-
| `payload.Filters = ["<field>:<value>"]` | `Filters` | top-level doc fields |
87-
| `payload.FiltersAll = ["<field>:<value>"]` | `FiltersAll` | top-level doc fields (AND semantics) |
88-
| `payload.Name = "<value>"` | `Name` | `name` (text search) |
89-
| `payload.DateField` / `DateFrom` / `DateTo` | date range | date fields |
84+
| Mechanism | Query service field | Index field |
85+
|---------------------------------------------|---------------------|--------------------------------------|
86+
| `payload.Parent = "<type>:<uid>"` | `Parent` | `parent_refs` |
87+
| `payload.Tags = ["<key>:<value>"]` | `Tags` | `tags` |
88+
| `payload.Filters = ["<field>:<value>"]` | `Filters` | top-level doc fields |
89+
| `payload.FiltersAll = ["<field>:<value>"]` | `FiltersAll` | top-level doc fields (AND semantics) |
90+
| `payload.Name = "<value>"` | `Name` | `name` (text search) |
91+
| `payload.DateField` / `DateFrom` / `DateTo` | date range | date fields |
9092

9193
Only `Parent`, `Tags`, `Filters`, and `FiltersAll` are structural filters that
9294
map to indexed fields — these are the ones to validate. `Name` and date fields
@@ -95,33 +97,33 @@ are query-time text/range operations and do not need index field verification.
9597
Reference table of known tools and their structured filter parameters (verify
9698
against the grep output above before trusting this):
9799

98-
| Tool | Resource type | Parameter | Mechanism | Sent as |
99-
|---|---|---|---|---|
100-
| `search_projects` | `project` | `parent_uid` | Parent | `project:<uid>` |
101-
| `search_committees` | `committee` | `project_uid` | Parent | `project:<uid>` |
102-
| `search_committee_members` | `committee_member` | `committee_uid` | Tag | `committee_uid:<uid>` |
103-
| `search_committee_members` | `committee_member` | `project_uid` | Tag | `project_uid:<uid>` |
104-
| `search_mailing_lists` | `groupsio_mailing_list` | `project_uid` | Parent | `project:<uid>` |
105-
| `search_mailing_list_members` | `groupsio_member` | `mailing_list_id` | Tag | `mailing_list_uid:<id>` |
106-
| `search_mailing_list_members` | `groupsio_member` | `project_uid` | Tag | `project_uid:<uid>` |
107-
| `search_meetings` | `v1_meeting` | `committee_uid` | Parent (preferred) | `committee:<uid>` |
108-
| `search_meetings` | `v1_meeting` | `project_uid` | Parent (fallback) | `project:<uid>` |
109-
| `search_meeting_registrants` | `v1_meeting_registrant` | `meeting_id` | Parent (preferred) | `meeting:<id>` |
110-
| `search_meeting_registrants` | `v1_meeting_registrant` | `committee_uid` | Parent (fallback) | `committee:<uid>` |
111-
| `search_past_meetings` | `v1_past_meeting` | `project_uid` | Parent | `project:<uid>` |
112-
| `search_past_meetings` | `v1_past_meeting` | `committee_uid` | Tag | `committee_uid:<uid>` |
113-
| `search_past_meetings` | `v1_past_meeting` | `meeting_id` | Tag | `meeting_id:<id>` |
114-
| `search_past_meeting_participants` | `v1_past_meeting_participant` | `past_meeting_id` | Parent (preferred) | `past_meeting:<meeting_and_occurrence_id>` |
115-
| `search_past_meeting_participants` | `v1_past_meeting_participant` | `project_uid` | Parent (fallback) | `project:<uid>` |
116-
| `search_past_meeting_summaries` | `v1_past_meeting_summary` | `past_meeting_id` | Parent (preferred) | `past_meeting:<meeting_and_occurrence_id>` |
117-
| `search_past_meeting_summaries` | `v1_past_meeting_summary` | `project_uid` | Parent (fallback) | `project:<uid>` |
118-
| `search_members` | `project_membership` | `project_uid` | FiltersAll | `project_uid:<uid>` |
119-
| `search_members` | `project_membership` | `b2b_org_uid` | FiltersAll | `b2b_org_uid:<uid>` |
120-
| `search_members` | `project_membership` | `tier_uid` | FiltersAll | `tier_uid:<uid>` |
121-
| `search_members` | `project_membership` | `tier_name` | FiltersAll | `tier_name:<name>` |
122-
| `search_members` | `project_membership` | `status` | FiltersAll | `status:Active` (hardcoded default) |
123-
| `get_membership_key_contacts` | `key_contact` | `membership_uid` | FiltersAll | `membership_uid:<uid>` |
124-
| `search_b2b_orgs` | `b2b_org` | *(none — Name only)* |||
100+
| Tool | Resource type | Parameter | Mechanism | Sent as |
101+
|------------------------------------|-------------------------------|----------------------|--------------------|--------------------------------------------|
102+
| `search_projects` | `project` | `parent_uid` | Parent | `project:<uid>` |
103+
| `search_committees` | `committee` | `project_uid` | Parent | `project:<uid>` |
104+
| `search_committee_members` | `committee_member` | `committee_uid` | Tag | `committee_uid:<uid>` |
105+
| `search_committee_members` | `committee_member` | `project_uid` | Tag | `project_uid:<uid>` |
106+
| `search_mailing_lists` | `groupsio_mailing_list` | `project_uid` | Parent | `project:<uid>` |
107+
| `search_mailing_list_members` | `groupsio_member` | `mailing_list_id` | Tag | `mailing_list_uid:<id>` |
108+
| `search_mailing_list_members` | `groupsio_member` | `project_uid` | Tag | `project_uid:<uid>` |
109+
| `search_meetings` | `v1_meeting` | `committee_uid` | Parent (preferred) | `committee:<uid>` |
110+
| `search_meetings` | `v1_meeting` | `project_uid` | Parent (fallback) | `project:<uid>` |
111+
| `search_meeting_registrants` | `v1_meeting_registrant` | `meeting_id` | Parent (preferred) | `meeting:<id>` |
112+
| `search_meeting_registrants` | `v1_meeting_registrant` | `committee_uid` | Parent (fallback) | `committee:<uid>` |
113+
| `search_past_meetings` | `v1_past_meeting` | `project_uid` | Parent | `project:<uid>` |
114+
| `search_past_meetings` | `v1_past_meeting` | `committee_uid` | Tag | `committee_uid:<uid>` |
115+
| `search_past_meetings` | `v1_past_meeting` | `meeting_id` | Tag | `meeting_id:<id>` |
116+
| `search_past_meeting_participants` | `v1_past_meeting_participant` | `past_meeting_id` | Parent (preferred) | `past_meeting:<meeting_and_occurrence_id>` |
117+
| `search_past_meeting_participants` | `v1_past_meeting_participant` | `project_uid` | Parent (fallback) | `project:<uid>` |
118+
| `search_past_meeting_summaries` | `v1_past_meeting_summary` | `past_meeting_id` | Parent (preferred) | `past_meeting:<meeting_and_occurrence_id>` |
119+
| `search_past_meeting_summaries` | `v1_past_meeting_summary` | `project_uid` | Parent (fallback) | `project:<uid>` |
120+
| `search_members` | `project_membership` | `project_uid` | FiltersAll | `project_uid:<uid>` |
121+
| `search_members` | `project_membership` | `b2b_org_uid` | FiltersAll | `b2b_org_uid:<uid>` |
122+
| `search_members` | `project_membership` | `tier_uid` | FiltersAll | `tier_uid:<uid>` |
123+
| `search_members` | `project_membership` | `tier_name` | FiltersAll | `tier_name:<name>` |
124+
| `search_members` | `project_membership` | `status` | FiltersAll | `status:Active` (hardcoded default) |
125+
| `get_membership_key_contacts` | `key_contact` | `membership_uid` | FiltersAll | `membership_uid:<uid>` |
126+
| `search_b2b_orgs` | `b2b_org` | *(none — Name only)* | | |
125127

126128
## Step 3 — Fetch indexer contracts
127129

@@ -135,13 +137,13 @@ Known contract URLs:
135137
- `v1_meeting`, `v1_meeting_registrant`, `v1_past_meeting`,
136138
`v1_past_meeting_participant`, `v1_past_meeting_transcript`,
137139
`v1_past_meeting_summary`:
138-
https://github.com/linuxfoundation/lfx-v2-meeting-service/blob/main/docs/indexer-contract.md
140+
<https://github.com/linuxfoundation/lfx-v2-meeting-service/blob/main/docs/indexer-contract.md>
139141
- `project`:
140-
https://github.com/linuxfoundation/lfx-v2-project-service/blob/main/docs/indexer-contract.md
142+
<https://github.com/linuxfoundation/lfx-v2-project-service/blob/main/docs/indexer-contract.md>
141143
- `committee`, `committee_member`:
142-
https://github.com/linuxfoundation/lfx-v2-committee-service/blob/main/docs/indexer-contract.md
144+
<https://github.com/linuxfoundation/lfx-v2-committee-service/blob/main/docs/indexer-contract.md>
143145
- `groupsio_mailing_list`, `groupsio_member`:
144-
https://github.com/linuxfoundation/lfx-v2-mailing-list-service/blob/main/docs/indexer-contract.md
146+
<https://github.com/linuxfoundation/lfx-v2-mailing-list-service/blob/main/docs/indexer-contract.md>
145147

146148
Fetch each URL and extract the **Tags** table and **Parent References** table
147149
for each resource type. Record which tag keys and parent_ref prefixes the
@@ -254,11 +256,11 @@ matches what the contract specifies.
254256
```markdown
255257
## <tool_name> (resource type: <type>)
256258

257-
| Parameter | Mechanism | Sent as | Contract | Index evidence | Verdict |
258-
|---|---|---|---|---|---|
259-
| committee_uid | Parent | committee:<uid> | ✅ parent_ref `committee:` | parent_refs prefix "committee:" — N hits | ✅ Works |
260-
| project_uid | Parent | project:<uid> | ✅ parent_ref `project:` | parent_refs prefix "project:" — 0 hits | ⚠️ Broken |
261-
| meeting_id | Tag | meeting_id:<id> | ⚠️ not in contract | tag key "meeting_id:" — N hits | ⚠️ Review |
259+
| Parameter | Mechanism | Sent as | Contract | Index evidence | Verdict |
260+
|---------------|-----------|-----------------|---------------------------|------------------------------------------|-----------|
261+
| committee_uid | Parent | committee:<uid> | ✅ parent_ref `committee:` | parent_refs prefix "committee:" — N hits | ✅ Works |
262+
| project_uid | Parent | project:<uid> | ✅ parent_ref `project:` | parent_refs prefix "project:" — 0 hits | ⚠️ Broken |
263+
| meeting_id | Tag | meeting_id:<id> | ⚠️ not in contract | tag key "meeting_id:" — N hits | ⚠️ Review |
262264
```
263265

264266
After the table, state explicitly:

.github/copilot-instructions.md

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
# Copilot Instructions — LFX MCP Server
2+
3+
## What This Repository Does
4+
5+
This is the **LFX MCP Server** — a Model Context Protocol (MCP) implementation in Go that exposes the Linux Foundation's LFX platform as MCP tools for AI agents. It uses the official MCP Go SDK and supports JSON-RPC 2.0 over stdio (dev) and Streamable HTTP (production) transports.
6+
7+
**Key facts**: ~25 Go source files, single binary, no database, stateless HTTP mode. Go and MCP SDK versions are pinned in `go.mod`. Built with `ko` for container images. Deployed to Kubernetes via Helm.
8+
9+
---
10+
11+
## CI Rules (what breaks the build)
12+
13+
Two workflows run on every PR (`.github/workflows/`):
14+
15+
1. **`license-header-check.yml`** — tracked source/config files (Go, YAML, shell, Makefile, etc.) must begin with the license header.
16+
2. **`mega-linter.yml`** — MegaLinter Go flavor v9 (config: `.mega-linter.yml`).
17+
18+
### Requirements for new/modified files
19+
20+
**License header** — always include as the first lines:
21+
22+
```go
23+
// Copyright The Linux Foundation and contributors.
24+
// SPDX-License-Identifier: MIT
25+
```
26+
27+
For YAML, shell, and Makefile use `#` comment syntax. Missing headers are the #1 cause of CI failure.
28+
29+
**Package doc comment** — every non-test `.go` file must have `// Package <name> ...` immediately above the `package` declaration. The `GO_REVIVE` linter enforces the `package-comments` rule. `_test.go` files are exempt.
30+
31+
**YAML formatting** — keep lines ≤120 characters (config: `.yamllint`); this is a warning, not a build failure. Helm templates in `charts/lfx-mcp/templates/` are excluded from YAML linting.
32+
33+
**Other MegaLinter checks:** the Go flavor bundles a full set of Go and common scripting/CI linters (shell, Docker, GitHub Actions, Helm, Markdown, secrets, etc.) and runs them all by default. `.mega-linter.yml` is the single source of truth for what's disabled or downgraded to error-only — check it rather than relying on any list here.
34+
35+
---
36+
37+
## Project Layout
38+
39+
```text
40+
cmd/lfx-mcp-server/main.go — Entry point, config, flag parsing, tool registration
41+
internal/tools/ — All MCP tool implementations (one file per tool/domain)
42+
internal/tools/scopes.go — Scope constants (ScopeRead, ScopeManage)
43+
internal/tools/helpers.go — Shared utilities for tool handlers
44+
internal/auth/ — JWT verification, API-key verification
45+
internal/lfxv2/ — LFX V2 API client (token exchange, slug resolver, access checks)
46+
internal/serviceapi/ — Generic HTTP client for downstream service APIs
47+
internal/otel/ — OpenTelemetry initialization
48+
charts/lfx-mcp/ — Helm chart (deployment, ingress, service, PDB)
49+
Makefile — Build automation (targets: build, test, check, clean, etc.)
50+
.mega-linter.yml — MegaLinter config
51+
.yamllint — YAML lint rules (max line length: 120)
52+
.ko.yaml — ko builder config with ldflags
53+
Dockerfile — Multi-stage build (Chainguard base images)
54+
AGENTS.md — Detailed developer guide (canonical; CLAUDE.md is a symlink)
55+
ARCHITECTURE.md — System architecture with Mermaid diagrams
56+
```
57+
58+
---
59+
60+
## Adding or Modifying Tools
61+
62+
Each tool lives in `internal/tools/<domain>.go`. The pattern is:
63+
64+
1. Define an args struct with `json` + `jsonschema` tags.
65+
2. Write a `Register<ToolName>(server *mcp.Server)` function that calls `mcp.AddTool`.
66+
3. Write a `handle<ToolName>` function implementing the logic.
67+
4. Register the tool in `cmd/lfx-mcp-server/main.go` inside `newServer()`, gated on `canRead` or `canManage`.
68+
5. Add the tool name to the `defaultTools` slice (also in `main.go`) if it should be enabled by default.
69+
70+
Tool annotations: always set `ReadOnlyHint: true` for read tools. Write tools must explicitly set `DestructiveHint`.
71+
72+
---
73+
74+
## Key Conventions
75+
76+
- **Package comments**: Every non-test `.go` file needs a `// Package <name> ...` comment (revive enforces this; `_test.go` files are exempt).
77+
- **Error constant**: Use `const errKey = "error"` for structured logging error keys.
78+
- **Logging**: Use `slog` (Go stdlib). Debug-only logs use `slog.Debug(...)`.
79+
- **No wrapper functions for scope enforcement** — tool gating is done inline in `newServer()`.
80+
- **JSON schema generation**: The MCP SDK auto-generates schemas from struct tags; no manual schema files.
81+
- **`schemaCache`**: A package-level cache shared across per-request server instances; do not duplicate it.
82+
83+
---
84+
85+
## Common Pitfalls
86+
87+
- Forgetting the license header on new files is the #1 cause of CI failure.
88+
- New non-test `.go` files without a package doc comment will fail the revive `package-comments` rule.
89+
- The `defaultTools` list in `main.go` controls which tools are enabled by default; adding a Register call without adding the name to `defaultTools` means the tool won't run unless explicitly enabled via `-tools`/`LFXMCP_TOOLS`.
90+
- Helm chart templates (`charts/lfx-mcp/templates/`) are excluded from YAML linting via regex in `.mega-linter.yml`.
91+
92+
---
93+
94+
## Trust These Instructions
95+
96+
These instructions are validated and current. Only perform additional exploration if the information above is incomplete or produces errors. For detailed architecture, tool patterns, and environment variable reference, consult `AGENTS.md` in the repo root.

0 commit comments

Comments
 (0)