Skip to content

Commit 02b666d

Browse files
dealakodealako
authored
Merge branch 'main' into feature/project-summary
2 parents b048e3a + 79445a6 commit 02b666d

25 files changed

Lines changed: 2150 additions & 1773 deletions

File tree

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

Lines changed: 355 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,355 @@
1+
---
2+
name: upgrade-maintenance
3+
description: Upgrade all Go dependencies, fix payload/contract changes introduced by upstream LFX V2 services, and run a query service argument review pass. Use when upgrading dependencies, after upstream service contract changes, or for periodic maintenance of the lfx-mcp codebase.
4+
license: MIT
5+
---
6+
7+
# Upgrade Maintenance
8+
9+
Perform a full upgrade maintenance pass on the lfx-mcp codebase. This covers
10+
three sequential phases: dependency upgrades, upstream contract fixes, and a
11+
query service argument review.
12+
13+
## Phase 1 — Upgrade all dependencies
14+
15+
### Step 1.1 — Upgrade the Go toolchain version
16+
17+
Compare the local toolchain version against the `go` directive in `go.mod` and
18+
update if the local toolchain is newer:
19+
20+
```bash
21+
# Extract the version from the local toolchain (e.g. "go1.24.3").
22+
LOCAL=$(go version | awk '{print $3}') # e.g. go1.24.3
23+
GOMOD=$(grep '^go ' go.mod | awk '{print $2}') # e.g. 1.23.0
24+
25+
echo "Local toolchain: $LOCAL"
26+
echo "go.mod go directive: $GOMOD"
27+
```
28+
29+
If the local toolchain is newer, update `go.mod`:
30+
31+
```bash
32+
# Updates the go directive to match the local toolchain.
33+
go mod tidy -go=${LOCAL#go}
34+
```
35+
36+
Also update any other files that pin the Go version:
37+
38+
- `Dockerfile` — `FROM golang:X.Y.Z` or `FROM golang:X.Y-alpine` base image.
39+
- `.github/workflows/*.yml` — `go-version:` fields in GitHub Actions steps.
40+
- `Makefile` — any hardcoded `GO_VERSION` variable.
41+
- `AGENTS.md` / `README.md` — version references in prerequisite tables.
42+
43+
After updating, verify the build still compiles with the new toolchain:
44+
45+
```bash
46+
make build
47+
```
48+
49+
If any deprecated language features or removed stdlib symbols cause errors,
50+
fix them before continuing.
51+
52+
### Step 1.2 — Upgrade Go module dependencies
53+
54+
```bash
55+
# Upgrade all direct dependencies to their latest minor/patch versions.
56+
go get -u ./...
57+
58+
# Tidy the module graph (removes unused indirect deps, syncs go.sum).
59+
go mod tidy
60+
```
61+
62+
After running, inspect `go.mod` for version changes. The most impactful upgrades
63+
to watch are the **`github.com/linuxfoundation/lfx-v2-*` packages** — they
64+
regularly add, rename, or remove Goa-generated struct fields, which directly
65+
affects tool handler code in `internal/tools/` and client wiring in
66+
`internal/lfxv2/client.go`.
67+
68+
All currently connected LFX services can be discovered from `go.mod`:
69+
70+
```bash
71+
grep 'linuxfoundation/lfx-v2-' go.mod
72+
```
73+
74+
Two categories of service are present, with different upgrade patterns:
75+
76+
- **Goa CRUD services** (all `lfx-v2-*` packages except query): each exposes
77+
standard resource CRUD operations via a Goa-generated HTTP client. The
78+
upgrade risks are: (1) new RPCs added → `NewClient` gains extra positional
79+
arguments; (2) response struct fields added/removed/renamed → tool handlers
80+
and view mapping functions need updating.
81+
- **Query service** (`lfx-v2-query-service`): a generic search interface; its
82+
payload struct (`Parent`, `Tags`, `Filters`) is stable but the string values
83+
sent (tag keys, parent-ref prefixes, resource type names) are driven by
84+
upstream indexer contracts, reviewed in Phase 2.
85+
- **MCP SDK** (`github.com/modelcontextprotocol/go-sdk`): less common; only
86+
minor/patch bumps expected. A major bump could change `mcp.AddTool`
87+
signatures, annotation fields, or content types.
88+
89+
### Step 1.2.1 — Update the OTel semconv import version
90+
91+
`go.opentelemetry.io/otel/semconv` is **not** a separate Go module. The version
92+
in the import path (e.g. `semconv/v1.40.0`) is an OTel semantic-convention spec
93+
version that is a subdirectory **inside** the `go.opentelemetry.io/otel` module.
94+
95+
**Why this matters**: instrumentation packages (e.g. `otelhttp`) always import
96+
the semconv sub-package they were written against. When that version differs from
97+
the one imported in our own code, the OTel SDK emits a startup error:
98+
99+
```text
100+
conflicting Schema URL: https://opentelemetry.io/schemas/1.41.0 and
101+
https://opentelemetry.io/schemas/1.40.0
102+
```
103+
104+
**Rule**: after upgrading `go.opentelemetry.io/otel`, always update our semconv
105+
import paths to the **highest-numbered sub-package present** inside the new otel
106+
version. That is deterministically the latest spec version shipped with the
107+
module, and it is the version the upgraded instrumentation packages will register.
108+
109+
Find it with:
110+
111+
```bash
112+
OTEL_VER=$(grep 'go.opentelemetry.io/otel v' go.mod | grep -v '//' | awk '{print $2}' | head -1)
113+
ls $(go env GOMODCACHE)/go.opentelemetry.io/otel@${OTEL_VER}/semconv/ \
114+
| grep '^v[0-9]' | sort -V | tail -1
115+
# e.g. "v1.41.0"
116+
```
117+
118+
Then update every `semconv/v*` import in the codebase to use that version:
119+
120+
```bash
121+
# Find all semconv imports.
122+
grep -rn 'semconv/v' internal/ cmd/ --include='*.go'
123+
```
124+
125+
Update each import path in-place (e.g. `semconv/v1.40.0` → `semconv/v1.41.0`).
126+
The attributes and helper functions are additive across minor spec versions, so
127+
the rename is always safe for a minor/patch upgrade.
128+
129+
After updating, verify the build and confirm the schema-URL conflict is gone:
130+
131+
```bash
132+
make build && ./scripts/test_server.sh 2>&1 | grep -i 'conflicting\|schema'
133+
# Should produce no output.
134+
```
135+
136+
### Step 1.3 — Verify the build compiles
137+
138+
```bash
139+
make build
140+
```
141+
142+
If the build fails, read the compiler errors before proceeding. Common causes
143+
after Goa CRUD service upgrades:
144+
145+
- **`NewClient` has too few arguments** — a service added new RPCs. Every Goa
146+
CRUD service wires its `NewClient` call in `internal/lfxv2/client.go`. Find
147+
the new method names on the generated HTTP client (look in the
148+
`gen/http/<service>/client/client.go` file of the upgraded module) and pass
149+
them as the additional positional arguments. Use `nil` for encoder-function
150+
arguments on endpoints you don't call (e.g. multipart upload endpoints).
151+
- **Struct field no longer exists** — a service removed a Goa response field.
152+
Find all usages in `internal/tools/` and `internal/lfxv2/` with `grep`.
153+
If the code was a deliberate workaround for a known upstream bug (look for
154+
comments referencing a Jira ticket like `// see LFXV2-XXXX`), the workaround
155+
can likely be deleted entirely now that the upstream fix is in. If it was
156+
real business logic, update accordingly.
157+
- **Renamed or removed struct fields in the MCP SDK** — rare, but check
158+
`ToolAnnotations`, handler signatures, and content type constructors.
159+
160+
Fix all compilation errors, then re-run `make build` until clean.
161+
162+
### Step 1.4 — Run the integration test suite
163+
164+
```bash
165+
./scripts/test_server.sh
166+
```
167+
168+
If tests fail, diagnose from the JSON-RPC output. Most failures after LFX
169+
service upgrades are field-shape mismatches in tool response JSON. Fix and
170+
re-run until all tests pass.
171+
172+
---
173+
174+
## Phase 2 — Fix upstream LFX V2 service contract changes
175+
176+
Upstream Goa CRUD services ship indexer contracts documenting their tag keys,
177+
parent_ref prefixes, and resource field shapes. The query service uses these
178+
values as string literals in tool handlers. When a service changes its
179+
contract, those string literals and view mapping functions may need updating.
180+
181+
**The most common pattern** is a Goa response struct field being removed or
182+
renamed. Any code in `internal/tools/` that references the field will fail to
183+
compile — the build error from Phase 1.3 is the signal. Less visibly, **new
184+
fields are silently dropped** by the explicit `to*View` mapping functions that
185+
filter upstream response structs into MCP output (e.g. `toB2bOrgMembershipView`,
186+
`toMembershipView`, `toKeyContactView`).
187+
188+
### Step 2.1 — Discover which services changed and what moved
189+
190+
For each upgraded Goa CRUD service (everything except query), diff its response
191+
struct fields between the old and new version. The old and new versions come
192+
from `git diff go.mod`. Use `$(go env GOMODCACHE)` to locate modules:
193+
194+
```bash
195+
MODCACHE=$(go env GOMODCACHE)
196+
SVC=github.com/linuxfoundation/lfx-v2-SERVICENAME
197+
OLD=vX.Y.Z # from git diff go.mod
198+
NEW=vX.Y.Z+1
199+
200+
diff \
201+
<(find "${MODCACHE}/${SVC}@${OLD}" -name '*.go' \
202+
| xargs grep -h -A60 'type.*Response.*struct\|type.*Result.*struct' 2>/dev/null \
203+
| grep -E '^\s+[A-Z]\w+\s') \
204+
<(find "${MODCACHE}/${SVC}@${NEW}" -name '*.go' \
205+
| xargs grep -h -A60 'type.*Response.*struct\|type.*Result.*struct' 2>/dev/null \
206+
| grep -E '^\s+[A-Z]\w+\s') \
207+
| grep '^[<>]'
208+
```
209+
210+
Run this for every upgraded `lfx-v2-*` service (skip the query service).
211+
212+
Also check for stale workarounds anywhere in `internal/`:
213+
214+
```bash
215+
grep -rn 'LFXV2-\|// Mask\|// Strip\|// workaround' internal/
216+
```
217+
218+
### Step 2.2 — Act on each finding
219+
220+
Apply the appropriate fix for each change found:
221+
222+
**Removed field** (compile error): delete all references in `internal/tools/`
223+
and `internal/lfxv2/`. If the code was a workaround (Jira comment present),
224+
delete the entire workaround block. If it was real business logic, update the
225+
handler to work without the field.
226+
227+
**New field on a response struct used by a `to*View` mapper**: decide whether
228+
the field is useful to MCP clients. If yes, add it to both the `*View` struct
229+
(with a `json:"..."` tag) and the `to*View` function. If not, add a comment on
230+
the view struct explaining why it was excluded.
231+
232+
**New field on a response struct not covered by a view mapper** (returned
233+
directly as JSON): no action needed — the field will appear automatically.
234+
235+
**`NewClient` argument count mismatch** (compile error): already handled in
236+
Step 1.3. Included here as a reminder that it originates from new RPCs in a
237+
Goa CRUD service, not from a contract doc change.
238+
239+
**New field on an upstream payload struct (create/update handlers)**: the
240+
compiler does not require exhaustive struct literal initialization, so new
241+
fields are silently zeroed and never sent. Two patterns exist in
242+
`internal/tools/` and both must be checked after any Goa CRUD service upgrade:
243+
244+
- *Pattern 1 — MCP args adapter with field renaming*: an adapter function that
245+
delegates to another handler using a struct literal with explicit field
246+
mappings (e.g. `handleCreateGroupMemberMode` →
247+
`CreateCommitteeMemberArgs{CommitteeUID: args.GroupUID, ...}`). These use
248+
struct literals because the field names differ between source and destination
249+
types, so a type conversion is not possible. A new field on the destination
250+
type will be silently zeroed.
251+
252+
Note: adapters where source and destination types are structurally identical
253+
(same field names and types) should use a direct type conversion
254+
(`DestType(args)`) instead of a struct literal — the compiler then enforces
255+
exhaustiveness and this check is not needed for those.
256+
257+
- *Pattern 2 — Upstream payload builder*: a handler that constructs an upstream
258+
Goa payload struct directly, typically in two parts: a baseline struct literal
259+
seeded from current state, then a block of `if args.X != nil { payload.X = …
260+
}` overrides. A new upstream payload field can be silently dropped from
261+
either part.
262+
263+
To find candidates needing review:
264+
265+
```bash
266+
# Find adapter functions that still use struct literals (Pattern 1).
267+
# Type-conversion adapters (e.g. CreateCommitteeArgs(args)) are safe to skip.
268+
grep -n 'func handle.*Mode\b' internal/tools/*_write.go internal/tools/*.go 2>/dev/null
269+
# Then for each, check if the body uses a struct literal (not a type conversion).
270+
grep -A5 'func handle.*Mode\b' internal/tools/*_write.go internal/tools/*.go 2>/dev/null | grep 'Args{'
271+
272+
# Find upstream payload struct literals (Pattern 2).
273+
grep -n 'Payload{$\|Payload{' internal/tools/*_write.go
274+
```
275+
276+
For each match, compare the fields in the struct literal against the current
277+
field list of the destination type (check the upgraded module in
278+
`$(go env GOMODCACHE)`). For every field present on the destination type but
279+
absent from the literal, decide: forward it (add the mapping), or add a comment
280+
explaining why it is intentionally omitted.
281+
282+
**Tag key, parent_ref prefix, or resource type name changed** (query service
283+
string literals): update the affected string literals in `internal/tools/`.
284+
Cross-reference against the service's indexer contract to confirm the new
285+
value. Contracts live at:
286+
`https://github.com/linuxfoundation/lfx-v2-SERVICENAME/blob/main/docs/indexer-contract.md`
287+
288+
The filter-to-mechanism mapping for query service payloads:
289+
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 |
295+
296+
### Step 2.3 — Verify
297+
298+
After all fixes, run:
299+
300+
```bash
301+
make build && ./scripts/test_server.sh
302+
```
303+
304+
Both must pass before proceeding to Phase 3.
305+
306+
---
307+
308+
## Phase 3 — Query service argument review
309+
310+
Run the full `validate-search-filters` skill to cross-validate every filter
311+
parameter in every tool against the live OpenSearch index and the now-updated
312+
contracts.
313+
314+
> **Instruction to the agent**: load and execute the `validate-search-filters`
315+
> skill now. Complete all steps (1 through 6) and produce the full report.
316+
> Only apply fixes (Step 7) if explicitly instructed by the user.
317+
318+
The output of the validate-search-filters skill drives any remaining filter
319+
corrections. If Step 7 fixes are applied there, re-run `make build` and
320+
`./scripts/test_server.sh` again after.
321+
322+
---
323+
324+
## Phase 4 — Final verification and wrap-up
325+
326+
### Step 4.1 — Full quality check
327+
328+
```bash
329+
make check
330+
```
331+
332+
Fix any lint or vet errors reported.
333+
334+
### Step 4.2 — Full build and test
335+
336+
```bash
337+
make build && ./scripts/test_server.sh
338+
```
339+
340+
Both must complete without errors before the maintenance pass is considered
341+
done.
342+
343+
### Step 4.3 — Summarise changes
344+
345+
Produce a short plain-text summary of:
346+
347+
1. Which dependencies were upgraded, noting any Goa CRUD service bumps and
348+
what structural changes (new RPCs, removed/added fields) required fixes.
349+
2. Which view mapping functions were updated and what new fields were included
350+
or explicitly excluded.
351+
3. The final validate-search-filters report (verdict table, any remaining ⚠️
352+
items).
353+
4. Any items that could not be fixed automatically and require human follow-up.
354+
355+
This summary is suitable for a PR description or Jira comment.

0 commit comments

Comments
 (0)