diff --git a/.gitignore b/.gitignore index 1c6f4ac6..5cac8e1c 100644 --- a/.gitignore +++ b/.gitignore @@ -61,3 +61,6 @@ coverage/ # agent .archive .agent +reports/ +.output.txt +poc/ diff --git a/cspell.config.json b/cspell.config.json index bc46682e..c82c2014 100644 --- a/cspell.config.json +++ b/cspell.config.json @@ -12,10 +12,12 @@ "onsessioninitialized", "onsessionclosed", "patternfly", + "prefault", "rereview", "rsort", "sparkline", "streamable", + "treeify", "unrepresentable", "unsub" ] diff --git a/guidelines/README.md b/guidelines/README.md index 228d056e..860c2cc0 100644 --- a/guidelines/README.md +++ b/guidelines/README.md @@ -19,6 +19,7 @@ Agent-specific guidelines for the PatternFly MCP project, optimized for machine ### Skills - [Add docs links](./skills/add-docs-links/SKILL.md) - Add documentation links to `src/docs.json` in a structured way (format, duplicate check, URL confirmation, tests) +- [Review Zod integration](./skills/review-zod-integration/SKILL.md) - Review Zod dependency bumps, map release notes to PF MCP, run tests, write `reports/YYYYMMDD-HHMMSS-zod-{semver}-update-report.md` (UTC) **Note:** `guidelines/skills/` is the canonical location for skills. Repo symlinks point here so agents can discover them: `.agents/skills` (Cursor), `.claude/skills` (Claude). The `.agent/` directory (no “s”) is reserved for each developer’s local work and is off limits—do not use it for shared skills or guidelines. @@ -36,6 +37,7 @@ Agents should use these phrases as signals to consult specific documentation and | **"review development guide"** | Review `docs/development.md` for CLI, API, and plugin authoring. | | **"create an example tool plugin"** | Review `guidelines/agent_coding.md`, `docs/development.md`, `docs/examples/*`, and `src/*` for context, coding standards, and existing example formats. | | **"add documentation links"** / **"add doc entries"** / **"register docs"** / **"update docs.json"** / **"contribute to docs.json"** | Follow `guidelines/skills/add-docs-links/SKILL.md`: docs.json format, duplicate check, raw URL confirmation, then run unit tests and update meta. | +| **"review zod"** / **"zod upgrade"** / **"zod integration review"** / **"zod update report"** | Follow `guidelines/skills/review-zod-integration/SKILL.md`: release notes vs codebase, tests, report at `reports/YYYYMMDD-HHMMSS-zod-{semver}-update-report.md` (UTC). | | **"troubleshoot server"** / **"debug server"** | Review `docs/usage.md#troubleshooting` and the PatternFly MCP server resource `patternfly://context` | ## Guidelines Processing Order diff --git a/guidelines/agent_coding.md b/guidelines/agent_coding.md index be541e24..a800af89 100644 --- a/guidelines/agent_coding.md +++ b/guidelines/agent_coding.md @@ -155,7 +155,7 @@ Centralized utilities in `server.helpers.ts`, `server.schema.ts`, and `server.ge - **Helpers**: Use `stringJoin.newline()`, `freezeObject()`, `timeoutFunction()`, and `mergeObjects()`. - **Schemas**: Use `normalizeInputSchema(schema)` to convert JSON Schema or Zod shapes into valid Zod instances. - **Type Guards**: Use `isPlainObject()`, `isZodSchema()`, and `isErrorLike()` for runtime narrowing. -- **Zod Detection**: Robustly detect Zod schemas by checking for internal brands: `_def` for Zod v3 and `_zod` for Zod v4. Avoid relying solely on `parse()` or `safeParse()` methods. +- **Zod Detection**: Robustly detect Zod schemas by checking for the public `.def` property (Zod v4+) or internal brands: `_zod` (v4) and `_def` (v3). Avoid relying solely on `parse()` or `safeParse()` methods. - **Immutability**: Options and Session objects are **frozen**; use `structuredClone()` before modification. ### 3.5 Validation and Sanitization diff --git a/guidelines/skills/add-docs-links/SKILL.md b/guidelines/skills/add-docs-links/SKILL.md index d51a36e5..76c9b9d1 100644 --- a/guidelines/skills/add-docs-links/SKILL.md +++ b/guidelines/skills/add-docs-links/SKILL.md @@ -7,7 +7,9 @@ description: Maintains the PatternFly MCP documentation catalog in src/docs.json ## When to use -User wants to **add**, **change**, or **remove** rows in `src/docs.json`, or fix broken / unreachable catalog links. They may paste **any GitHub URL** (blob or raw); you convert blob → raw, pick refs, verify reachability, shape entries, update `meta` and `generated`, and run tests. +- User wants to **add**, **change**, or **remove** rows in `src/docs.json`, or fix broken / unreachable catalog links +- User provides a GitHub URL (blob or raw) to register in the catalog — convert blob → raw, pick refs, verify reachability, shape entries, update `meta` and `generated`, and run tests +- User asks to update or fix `docs.json` entries, contribute to the docs catalog, or align with docs.json tests and CI audit For **edits or removals**: find the row by `path` and/or its `docs` key; update `meta` / `generated`; run tests. Same rules as adds: whitelist, unique `path`, and `src/__tests__/docs.json.test.ts` (including `baseHashes`). @@ -20,7 +22,7 @@ For **edits or removals**: find the row by `path` and/or its `docs` key; update - New ref for an existing repo: resolve SHA (e.g. commits API) or use a stable tag. 2. **Whitelist** - - `path` must match `patternflyOptions.urlWhitelist` in `src/options.defaults.ts`. See [reference.md](reference.md#url-whitelist-allowed-domains). Use **https** only. Do not widen the whitelist in a catalog-only PR. + - `path` must match `patternflyOptions.urlWhitelist` in `src/options.defaults.ts`. See [reference.md — URL whitelist](reference.md#url-whitelist-allowed-domains). Use **https** only. Do not widen the whitelist in a catalog-only PR. 3. **Reachability** - Response must be 2xx (e.g. `curl -sI -o /dev/null -w "%{http_code}" ""`). If not, fix ref or path; do not add a dead link. @@ -47,6 +49,10 @@ For **edits or removals**: find the row by `path` and/or its `docs` key; update **CI:** `.github/workflows/audit.yml` on PRs that touch `src/docs.json` or `tests/audit/**`. +## Additional resources + +- [reference.md](reference.md) — entry format, top-level structure, URL whitelist, duplicate check, ref lookup, unit test constraints, example entry + ## Quick checks - [ ] Blob → raw; ref reused per repo when possible. diff --git a/guidelines/skills/add-docs-links/reference.md b/guidelines/skills/add-docs-links/reference.md index 1110c82b..9dbf72f0 100644 --- a/guidelines/skills/add-docs-links/reference.md +++ b/guidelines/skills/add-docs-links/reference.md @@ -1,4 +1,4 @@ -# docs.json Reference +# Add docs links — Reference (PatternFly MCP) ## File location diff --git a/guidelines/skills/review-zod-integration/SKILL.md b/guidelines/skills/review-zod-integration/SKILL.md new file mode 100644 index 00000000..eb503ee3 --- /dev/null +++ b/guidelines/skills/review-zod-integration/SKILL.md @@ -0,0 +1,77 @@ +--- +name: review-zod-integration +description: Reviews Zod dependency upgrades for PatternFly MCP—maps release notes to codebase usage, runs tests, and writes a dated update report with impact tables and prioritized fixes. Use when bumping zod, on review zod, zod upgrade, zod integration review, assessing Zod breaking changes, generating a zod update report, or reviewing Dependabot/Renovate zod PRs. +--- + +# Review Zod Integration (PatternFly MCP) + +## When to use + +- User bumps or proposes bumping **`zod`** in `package.json` +- User asks for a **Zod upgrade review**, **integration review**, or **impact analysis** +- Dependabot/Renovate PR for `zod` +- User wants a report comparing **Zod release notes** to this codebase + +## Workflow + +1. **Versions and release notes** + - Read `package.json`, `package-lock.json`, and `node_modules/zod/package.json` if installed. + - Note **previous** version from `git log -1 -- package.json` or the bump PR/commit. + - Fetch release notes for the target minor/major (release page, GitHub API, packaged changelog, or any fetch tool): `https://github.com/colinhacks/zod/releases/tag/v{VERSION}`. + - For patch bumps within the same minor (e.g. 4.4.1 → 4.4.3), use the **minor** release notes (e.g. v4.4.0) plus patch-specific notes when present. + - **Offline fallback:** If release notes cannot be retrieved, state that limitation and continue from `package.json`, lockfile, and the inventory/audit steps only. + - Extract **potentially breaking**, **other fixes**, **performance**, and **locales** sections. + - If notes are ambiguous or the bump is minor/major, follow [reference.md — Audit depth policy](reference.md#audit-depth-policy) (prefer `node_modules/zod/src/` when shipped; otherwise published entry points from `package.json` `exports`). + +2. **Repo guidelines** + - Read (do not paste into the report): `CONTRIBUTING.md`, `guidelines/agent_coding.md`, `guidelines/agent_testing.md`, `docs/development.md`. + +3. **Codebase inventory** + - Run all commands in [reference.md — Grep patterns](reference.md#grep-patterns). Read [reference.md — Key files](reference.md#key-files). + - Record **used** vs **not used** per release-note item. + - Apply [reference.md — Compatibility policy (Zod detection)](reference.md#compatibility-policy-zod-detection): new detection must be **additive**; do not remove `_def` or v3 paths unless release notes show a concrete conflict. + +4. **Impact matrix** + - For **every** release-note bullet (breaking, fixes, performance, locales), add one row to the report tables. Columns: **Release note**, **Used in PF MCP?** (Yes / No / Indirect — cite file or grep), **Impact** (None / Low / Medium / High), **Priority**, **Recommended fix** (or `None`). Priority rules: [reference.md — Priority rules](reference.md#priority-rules-pf-mcp). + - Always include [reference.md — Updated P2 recommendations (Recurring)](reference.md#updated-p2-recommendations-recurring) in the report **Recommended fixes** section, even when tests pass. + - Row patterns: [reference.md — Example impact rows](reference.md#example-impact-rows-pf-mcp). + +5. **Tests** + From repo root: + + ```bash + npm run test:types + npx jest --selectProjects unit + npm run test:integration + npm run test:audit + ``` + + - Record pass/fail counts. Full `npm test` may fail on `auditor/` ESLint—treat as pre-existing unless the bump touched `auditor/`. + - **P0 (tests/types/runtime break):** Document failures in the report. Apply only minimal fixes needed to verify bump safety, re-run tests, and note any changes. Update snapshots only when output change is correct (`jest -u` scoped to affected files). + - **P1 / P2:** Do not change code during this review—list fixes in the report only. + +6. **Write the report** + - **Directory:** `reports/` at repository root (gitignored). Run `mkdir -p reports` if needed; do not delete prior reports. + - **Filename:** `YYYYMMDD-HHMMSS-zod-{semver}-update-report.md` (`YYYYMMDD-HHMMSS` = report timestamp in **UTC**, 24-hour, no separators in the time part; `{semver}` = target version without `v`, e.g. `reports/20260522-143045-zod-4.4.3-update-report.md`). + - Use [reference.md — Report template](reference.md#report-template). Do not commit unless the user asks. + +7. **User summary** + - Report path; **executive summary verdict table** from the report (code changes required, documentation updates required, risk level—do not collapse to a single line); P0/P1/P2 counts; link to release notes. + - Ask: *"Would you like me to proceed with the recommended P1/P2 fixes listed in the report?"* + - Do not modify the codebase further until the user explicitly grants permission (except minimal P0 fixes from step 5). + +Domain context (SDK routing, schema pipeline, peer range): [reference.md — Quick PF MCP facts](reference.md#quick-pf-mcp-facts). + +## Additional resources + +- [reference.md](reference.md) — grep patterns, key files, priority rules, compatibility policy, report template, example impact rows, architecture notes + +## Quick checks + +- [ ] Versions (from → to) and release notes captured (or offline limitation noted) +- [ ] Guidelines read; inventory grep run; compatibility policy applied +- [ ] Every release-note bullet mapped with priority +- [ ] Recurring P2s in report consolidated fixes +- [ ] `test:types`, unit, `test:integration`, `test:audit` recorded +- [ ] Report at `reports/YYYYMMDD-HHMMSS-zod-{semver}-update-report.md` (UTC) +- [ ] User asked before any P1/P2 code changes diff --git a/guidelines/skills/review-zod-integration/reference.md b/guidelines/skills/review-zod-integration/reference.md new file mode 100644 index 00000000..aa62db44 --- /dev/null +++ b/guidelines/skills/review-zod-integration/reference.md @@ -0,0 +1,293 @@ +# Review Zod integration — Reference (PatternFly MCP) + +## Quick PF MCP facts + +- **Zod is required for SDK routing:** Any tool registered with the MCP SDK must have a Zod `inputSchema` (even `z.any()`) so the SDK passes user arguments to the handler. Without it, handlers receive only a context object. +- Internal tools **require** Zod or raw Zod shapes; JSON Schema is converted via `src/server.schema.ts`. +- Core APIs: `fromJSONSchema`, `toJSONSchema`, `z.looseObject`, raw shapes with `.optional()` — not `z.undefined()`, `z.tuple()`, `.merge()`. +- MCP SDK peer: `zod ^3.25 || ^4.0`. + +## Priority rules (PF MCP) + +| Priority | When | +|----------|------| +| **P0** | Tests fail, types fail, or runtime/tool registration breaks after bump | +| **P1** | Used API with behavior change; snapshots or plugin manifests need update | +| **P2** | Optional hygiene (e.g. modern `.def` detection, docs for plugin authors). Do **not** remove legacy `_def` or v3-compatible detection unless the new Zod version contradicts it | +| **None** | Not used, or positive/neutral change with passing tests | + +## Key files + +| File | Zod role | +|------|----------| +| `src/server.schema.ts` | `isZodSchema`, `isZodRawShape`, `jsonSchemaToZod`, `normalizeInputSchema`, `zodToJsonSchema`; v3 `_def` / v4 `_zod` detection | +| `src/server.tools.ts` | `z.looseObject({})` Tools Host fallback | +| `src/server.toolsHost.ts` | Normalize plugin schemas; manifest JSON Schema via `zodToJsonSchema` | +| `src/server.toolsUser.ts` | `normalizeInputSchema` for inline/static tools | +| `src/options.assertions.ts` | `z.array(z.string().url().refine(...))` | +| `src/tool.patternFlyDocs.ts` | Raw Zod shape (`z.array`, `z.string`, `z.enum`, `.optional()`) | +| `src/tool.searchPatternFlyDocs.ts` | Same | +| `src/__tests__/server.schema.test.ts` | Conversion tests + `toJSONSchema` snapshots | +| `docs/development.md` | Plugin/tool schema terminology | +| `guidelines/agent_coding.md` | Zod detection guidance | + +## Grep patterns + +Canonical inventory commands (run all): + +```bash +# Zod imports and schema pipeline +grep -rE "from ['\"]zod['\"]|fromJSONSchema|toJSONSchema|isZodSchema|isZodRawShape|normalizeInputSchema|jsonSchemaToZod" src docs guidelines tests + +# Breaking-change-prone APIs +grep -rE "z\.(tuple|undefined|merge|base64|httpUrl|cuid|record|discriminatedUnion|function|map|set)|\.merge\(|\.passthrough|looseObject|\.prefault" src + +# Error formatting (snapshot risk) +grep -rE "ZodError|formatError|treeifyError" src +``` + +## APIs typically unused in PF MCP + +If release notes only mention these, impact is usually **None**: + +`z.tuple`, `z.undefined`, `.merge()`, `z.base64`, `z.httpUrl`, `z.cuid`, `z.record` (key transforms), `z.discriminatedUnion`, `z.function`, `z.map`/`z.set` with `.default()`, empty unions, custom Zod locales. + +## APIs in active use + +| API | Where | +|-----|--------| +| `fromJSONSchema` | `jsonSchemaToZod` | +| `toJSONSchema` | `zodToJsonSchema`, default `draft-2020-12` | +| `z.object` / raw shapes | Built-in tools, `normalizeInputSchema` | +| `z.looseObject` | `server.schema.ts`, `server.tools.ts`; Standard for "open" tool inputs in Zod 4.4+ | +| `z.string().url()` + `.refine` | `options.assertions.ts` | +| `z.enum`, `.optional()`, `.max()`, `.min()` | Tool input schemas | + +## Dependencies + +| Package | Notes | +|---------|--------| +| `zod` | Direct production dep; use native `fromJSONSchema` / `toJSONSchema` (not `zod-to-json-schema` in app code) | +| `@modelcontextprotocol/sdk` | Depends on / peers `zod ^3.25 \|\| ^4.0` | + +## Audit depth policy + +| Source | Frequency | Objective | +| :--- | :--- | :--- | +| **Release Notes** | Always | Identify public API changes and security fixes. | +| **Package Metadata** | Always | Verify peer dependencies and entry points. | +| **Source Code Audit** | Minor/Major Bumps | Validate internal state and undocumented behavioral shifts. | + +## Report template + +Save as: **`reports/YYYYMMDD-HHMMSS-zod-{semver}-update-report.md`** (`YYYYMMDD-HHMMSS` = UTC timestamp when the report is written; e.g. `reports/20260522-143045-zod-4.4.3-update-report.md`). The `reports/` directory is gitignored; ensure an allowance for an existing directory is made (e.g., `mkdir -p`) to preserve previous contents. + +```markdown +# Zod {version} Update Report — PatternFly MCP + +**Date:** YYYY-MM-DD +**Zod versions:** {from} → {to} +**Release reference:** [Zod v{X.Y.Z}](https://github.com/colinhacks/zod/releases/tag/v{X.Y.Z}) + +## Executive summary + +| Verdict | Detail | +|---------|--------| +| **Code changes required** | None / … | +| **Documentation updates required** | None / … | +| **Risk level** | Low / Medium / High | + +[1–2 sentences on overall safety.] + +--- + +## Test and build verification + +| Command | Result | +|---------|--------| +| `npm run test:types` | | +| `npx jest --selectProjects unit` | | +| `npm run test:integration` | | +| `npm run test:audit` | | + +--- + +## Dependency context + +| Package | Version | Zod relationship | +|---------|---------|------------------| +| `zod` | | | +| `@modelcontextprotocol/sdk` | | peer range | + +--- + +## Zod usage inventory (PatternFly MCP) + +| Location | Usage | +|----------|--------| +| | | + +### APIs not used in this repo + +[List from grep.] + +--- + +## Release notes vs. PatternFly MCP + +### Potentially breaking changes + +| Release note | Used in PF MCP? | Impact | Priority | Recommended fix | +|--------------|-----------------|--------|----------|-----------------| +| | Yes/No/Indirect | None/Low/Medium/High | P0/P1/P2/None | | + +### Other behavioral fixes + +| Release note | Used in PF MCP? | Impact | Priority | Recommended fix | +|--------------|-----------------|--------|----------|-----------------| +| | | | | | + +### Performance / packaging + +| Release note | Impact on PF MCP | Priority | Recommended fix | +|--------------|------------------|----------|-----------------| +| | | | | + +### Locales + +| Release note | Impact on PF MCP | Priority | Recommended fix | +|--------------|------------------|----------|-----------------| +| | | | | + +--- + +## Documentation and agent guidance + +| Asset | Status | +|-------|--------| +| `docs/development.md` | | +| `docs/examples/*` | | +| `guidelines/agent_coding.md` | | +| `tests/e2e/` | | + +--- + +## Recommended fixes (consolidated) + +Sorted by priority. + +### P0 — Must fix + +| Item | Recommended fix | +|------|-----------------| +| | | + +### P1 — Should fix + +| Item | Recommended fix | +|------|-----------------| +| | | + +### P2 — Optional + +| Item | Recommended fix | +|------|-----------------| +| | | + +--- + +## Conclusion + +[Final verdict.] + +--- + +## References + +- [Zod release notes](URL) +- Bump commit: `hash` (if applicable) +``` + +## Example impact rows (PF MCP) + +Use these as patterns when mapping **your** target release notes. Re-grep the codebase every review—do not assume rows copy forward unchanged. + +### Potentially breaking — sample rows + +| Release note | Used in PF MCP? | Impact | Priority | Recommended fix | +|--------------|-----------------|--------|----------|-----------------| +| Tuple defaults / dense optional tails | No — no `z.tuple()` | None | None | None | +| Required keys with `z.undefined()` | No — tools use `.optional()` on keys | None | None | None | +| `.merge()` throws when receiver has refinements | No — no `.merge()` | None | None | None | +| `toJSONSchema()` strips redundant `id` in `$defs` | Yes — `zodToJsonSchema`, Tools Host manifest | Low — no code reads `$defs.id` | None | None; re-run `server.schema` snapshots if manifest output changed | +| `z.httpUrl()` stricter URL validation | No — uses `z.string().url()` + `refine` in `options.assertions.ts` | None | None | None | +| Floating-point accuracy (`multipleOf`) | No — `multipleOf` not used in schemas | None | None | None | + +### Other fixes — sample rows + +| Release note | Used in PF MCP? | Impact | Priority | Recommended fix | +|--------------|-----------------|--------|----------|-----------------| +| `fromJSONSchema()` metadata / cyclic input handling | Yes — `jsonSchemaToZod` | Low positive — plugin conversion may differ | None | Monitor plugin edge cases; no code change if tests pass | +| **Metadata retention (round-trip preservation)** | Yes — `fromJSONSchema` / `toJSONSchema` | Low — snapshots may include extra keys | None | Update snapshots if custom keys are now preserved; no code change | +| `z.record()` key transforms | No | None | None | None | +| Skip `__proto__` in object catchall | Indirect — object parsing in tools/options | Low positive | None | None | +| Empty union construction | Indirect — via JSON Schema conversion | Low positive — safer conversion | None | None | + +### Performance / locales — sample rows + +| Release note | Impact on PF MCP | Priority | Recommended fix | +|--------------|------------------|----------|-----------------| +| Lazy-bound builder methods | Transparent runtime improvement | None | None | +| Locale message text changes | No custom Zod locale in repo | None | None | + +### Example executive summary (safe minor bump) + +```markdown +| Verdict | Detail | +|---------|--------| +| **Code changes required** | None | +| **Documentation updates required** | None | +| **Risk level** | Low | + +No release-note item maps to APIs used in PF MCP hot paths; unit, e2e, and audit tests pass. +``` + +### Example consolidated fixes (typical clean bump) + +```markdown +### P0 — Must fix +(none) + +### P1 — Should fix +(none) + +### P2 — Optional +| Item | Recommended fix | +|------|-----------------| +| v3 `passthrough()` fallback in `server.schema.ts` | **Keep** — tool plugins may use Zod v3 shapes; only revisit removal if PF MCP explicitly drops v3 plugin compatibility and pins Zod 4-only | +| Public `.def` detection | Add alongside `_zod` / `_def` in `isZodSchema` when relevant to the bump | +| Plugin authoring | Note in `docs/development.md`: prefer JSON Schema or Zod 4 for *new* plugins | +``` + +## Architecture + +### SDK routing trigger +The MCP SDK requires `inputSchema` to be a Zod instance to route `(args, context)` to tool handlers. We rehydrate minimal Zod in the parent process (via `normalizeInputSchema`) specifically to trigger this signature, while keeping genuine validation in the isolated child process. + +## Compatibility policy (Zod detection) + +To support a diverse plugin ecosystem, PatternFly MCP maintains a "Compatibility-First" approach for Zod schema detection. + +### Preservation rules +1. **Never Remove `_def` Detection**: Even when pinned to Zod 4+, the `isZodSchema` function in `src/server.schema.ts` must keep the `_def` check to support plugins using Zod v3. +2. **Additive Detection**: When Zod introduces new public APIs (like `.def` in v4.4), add detection for them *alongside* existing internal brands (`_zod`, `_def`). +3. **Exceptions**: Legacy detection should only be removed if the Zod release notes indicate that a legacy property (e.g., `_def`) has been repurposed in a way that causes false positives or crashes in the current version. + +### Updated P2 recommendations (recurring) +These items MUST be included in the "Recommended Fixes (P2)" section of the report for any Zod 4.4+ review to ensure long-term health, even if they are not strictly required for the current bump to "pass." + +- **DO**: Add detection for the public `.def` property (Zod 4.4+). +- **DO**: Update `docs/development.md` to recommend JSON Schema or Zod 4 for *new* plugins. +- **DO**: Migrate `z.string().url()` to `z.url()` where applicable (Zod 4.4+). +- **DO NOT**: Remove the v3 `_def` branch in `isZodSchema`. +- **DO NOT**: Remove the `passthrough()` fallback in `jsonSchemaToZod` while plugin/tool authors may supply Zod v3 schemas (even when the server pins Zod 4). Revisit only if PF MCP explicitly drops v3 plugin compatibility.