Skip to content

Commit a6b91fc

Browse files
committed
refactor: centralize MCP operation specs
1 parent a37e2d4 commit a6b91fc

47 files changed

Lines changed: 1182 additions & 488 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

PLAN.md

Lines changed: 131 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,33 @@
44

55
Prepare StreamNative MCP Server for Claude connector submission and review.
66

7+
## Current review follow-up: operation spec registry
8+
9+
Reviewer feedback is valid: current branch split read/write tool names, but duplicated `toolMode` helpers and per-builder write-operation maps still create scatter-shot maintenance. Adding one operation can require enum updates, write map updates, handler switch updates, docs, and compliance-test classification. `validateModeOperation` also classifies any operation missing from the write map as read, so an unclassified future write can pass read-mode validation until the handler switch rejects or mishandles it.
10+
11+
Recommended next design: make each tool family declare operation metadata once, then derive mode-specific tool schemas, annotations, validation, and tests from that registry.
12+
13+
```go
14+
type OperationMode string
15+
16+
const (
17+
OperationModeRead OperationMode = "read"
18+
OperationModeWrite OperationMode = "write"
19+
)
20+
21+
type OperationSpec struct {
22+
Name string
23+
Mode OperationMode
24+
Destructive bool
25+
Idempotent bool
26+
Resources []string
27+
Params []ParamSpec
28+
Handler OperationHandler
29+
}
30+
```
31+
32+
Scope is incremental but complete: add shared spec helpers, migrate `kafka/topics.go` and `pulsar/namespace.go` as reference implementations, then migrate every remaining split builder in batches. Keep current read/write tool names unchanged. Use docs generated operation-table blocks first; do not rewrite full Markdown documents.
33+
734
Hard requirements from Claude docs:
835

936
- every MCP tool has non-empty `annotations.title`
@@ -403,12 +430,16 @@ Plan:
403430

404431
### Phase 1: shared annotation + mode helpers
405432

433+
Status: implemented on current branch.
434+
406435
- Add `pkg/mcp/toolannotations` helper.
407436
- Add local read/write mode helpers in builders with mixed operations.
408437
- Add reusable operation validation helpers where a builder already has operation maps.
409438

410439
### Phase 2: split Kafka tools completely
411440

441+
Status: implemented on current branch; follow-up refactor still needed to remove duplicated operation classification.
442+
412443
- Update all Kafka builders to build mode-specific tools.
413444
- Ensure read/write tools have mode-specific descriptions, examples, operation enums, and parameter schemas.
414445
- Read-only config returns only read tools.
@@ -418,6 +449,8 @@ Plan:
418449

419450
### Phase 3: split Pulsar tools completely
420451

452+
Status: implemented on current branch; follow-up refactor still needed to remove duplicated operation classification.
453+
421454
- Update all Pulsar builders to build mode-specific tools.
422455
- Ensure read/write tools have mode-specific descriptions, examples, operation enums, and parameter schemas.
423456
- Preserve existing read-only behavior by not registering write tools in read-only config.
@@ -427,26 +460,110 @@ Plan:
427460

428461
### Phase 4: StreamNative Cloud/static tool annotations
429462

463+
Status: implemented on current branch.
464+
430465
- Add annotations to context/log/resource tools.
431466
- Keep already split apply/delete tools.
432467
- Ensure no new mixed resource tool appears.
433468

434469
### Phase 5: dynamic tools
435470

471+
Status: implemented on current branch.
472+
436473
- Add annotations to Functions-as-Tools.
437474
- Validate read-only exposure behavior.
438475

439476
### Phase 6: runtime-visible docs
440477

478+
Status: implemented on current branch, with generated operation-table guard tests for migrated split builders.
479+
441480
Update runtime-visible docs together with schema changes:
442481

443-
- `README.md` feature/tool examples if names change.
444-
- `docs/tools/*.md` matching renamed/split tools.
482+
- Keep current read/write tool names unchanged.
483+
- `README.md` feature/tool examples only if behavior changes.
484+
- `docs/tools/*.md` matching current split tools.
445485
- Split docs into explicit read/write sections where a family has both tool modes.
446486
- Ensure read docs do not mention write-only operations or parameters.
447487
- Ensure write docs do not rely on old mixed tool names.
448488
- Any design notes under `agents/` if tool surface changes are architectural.
449489

490+
### Phase 7: shared operation spec registry follow-up
491+
492+
Status: implemented on current branch.
493+
494+
Goal: make operation metadata single-source-of-truth and remove copy-paste `toolMode` + `writeOperations` maps.
495+
496+
1. Add shared operation metadata API, likely under `pkg/mcp/builders/operations.go`:
497+
- `OperationModeRead` / `OperationModeWrite`
498+
- `OperationSpec`
499+
- `ParamSpec`
500+
- `OperationHandler` type alias or adapter
501+
- `OperationRegistry` or helper functions over `[]OperationSpec`
502+
2. Shared helpers must derive:
503+
- read operation enum
504+
- write operation enum
505+
- operation description fragments/table rows
506+
- mode-specific validation
507+
- unknown-operation rejection
508+
- read/write annotation selection
509+
- compliance-test classification
510+
3. Validation semantics:
511+
- operation absent from spec => reject, never default to read
512+
- operation present with wrong mode => reject with mode-specific error
513+
- operation present with matching mode => dispatch allowed
514+
4. Migrate two reference builders first:
515+
- `pkg/mcp/builders/kafka/topics.go`
516+
- `pkg/mcp/builders/pulsar/namespace.go`
517+
5. Reference builder acceptance criteria:
518+
- no local `xxxWriteOperations` map
519+
- operation enum generated from specs
520+
- handler validation uses specs
521+
- unknown operation test added
522+
- read-only build still excludes write tools
523+
- tool names unchanged
524+
- docs operation table generated or checked from specs
525+
6. Migrate remaining Kafka split builders:
526+
- `connect.go`
527+
- `groups.go`
528+
- `schema_registry.go`
529+
- `topics.go`
530+
7. Migrate remaining Pulsar split builders:
531+
- `brokers.go`
532+
- `cluster.go`
533+
- `functions.go`
534+
- `namespace.go`
535+
- `nsisolationpolicy.go`
536+
- `packages.go`
537+
- `resourcequotas.go`
538+
- `schema.go`
539+
- `sinks.go`
540+
- `sources.go`
541+
- `subscription.go`
542+
- `tenant.go`
543+
- `topic.go`
544+
- `topic_policy.go`
545+
8. Keep pure read/write client tools out of forced registry migration unless helper reuse is cheap:
546+
- `kafka_client_produce`
547+
- `kafka_client_consume`
548+
- `pulsar_client_produce`
549+
- `pulsar_client_consume`
550+
- pure read Pulsar admin status/stats/worker tools
551+
9. Replace compliance-test manual classification:
552+
- derive write/read operation sets from specs
553+
- assert no enum mixes read/write specs
554+
- assert every enum value exists in specs
555+
- assert specs cover every handler switch operation
556+
10. Docs generation/check:
557+
- do not rewrite whole Markdown documents
558+
- add generated operation table blocks only:
559+
`<!-- generated:operations:start -->` / `<!-- generated:operations:end -->`
560+
- add `go generate` or test helper to refresh/check blocks
561+
11. Cleanup after migration:
562+
- delete or shrink duplicated Kafka/Pulsar `tool_mode.go`
563+
- move shared `pruneToolInputSchema`/required filtering helper if still duplicated
564+
- remove obsolete per-builder write maps
565+
- remove obsolete compliance-test write maps
566+
450567
## Tests / compliance guard
451568

452569
Add focused tests:
@@ -465,25 +582,36 @@ Add focused tests:
465582
- StreamNative Cloud/context/log/resource tools have valid annotations.
466583
- PFTools dynamic tool creation has valid annotation.
467584
- Operation validation rejects read operations on write tools and write operations on read tools.
585+
- Operation validation rejects unknown operations instead of treating them as read.
586+
- Operation enum values are derived from or checked against `OperationSpec`.
587+
- Compliance tests derive read/write classification from `OperationSpec`, not hand-maintained write maps.
588+
- Docs generated operation table blocks match `OperationSpec`.
468589

469590
Static guard:
470591

471592
- Build all feature sets and assert no `operation` enum contains both read and write verbs in one tool.
472593
- For split tool families, assert mode-specific schema/description purity with family-specific allow/deny lists.
594+
- Assert every handler switch operation has a matching `OperationSpec` entry.
473595

474596
## Risks
475597

476598
- Tool split is runtime-visible and likely breaking for clients/prompts that call old names.
477-
- Docs under `docs/tools/` can drift if not updated with split names.
599+
- Operation spec refactor can accidentally change schema ordering, descriptions, or enum content even when tool names stay unchanged.
600+
- Docs under `docs/tools/` can drift if generated operation blocks are not checked in tests or `go generate`.
478601
- Some operations are ambiguous (`consume`, `trigger`, cursor operations, context reset). Conservative destructive annotation may add confirmations but avoids unsafe auto-run.
479602
- Some current tools may have read-only-mode logic embedded in handlers; after split, registration and handler validation must both enforce mode to prevent write leakage.
603+
- `OperationSpec.Handler` can over-couple schema metadata and dispatch if introduced too early. Prefer enum/validation/test/doc generation first, then dispatch consolidation.
480604
- `mcp-go` default annotations are unsafe for compliance because title empty and destructive default true.
481605

482606
## Confirmed decisions
483607

484608
- Fix all current mixed read/write surfaces, not only `kafka/topics.go` and `pulsar/namespace.go`.
609+
- Implement operation-spec follow-up across all affected split builders, not only the two reference files.
610+
- Start with shared spec + two reference migrations: `kafka/topics.go` and `pulsar/namespace.go`.
611+
- Keep current read/write tool names unchanged.
485612
- Do not preserve old mixed tool names or old mixed builder/schema patterns.
486613
- Runtime-visible docs must be updated with read/write split and must avoid mixed read/write wording.
614+
- For docs generation, start with generated operation table blocks only; do not generate whole Markdown documents.
487615
- Conservative safety annotations are acceptable for ambiguous side-effect tools unless implementation proves true read-only behavior.
488616

489617
## Recommended validation

docs/tools/kafka_admin_connect.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
#### kafka-admin-connect
22

3+
4+
<!-- generated:operations:start -->
5+
| Tool | Mode | Operations |
6+
|---|---|---|
7+
| `kafka_admin_connect_read` | read | `list`, `get` |
8+
| `kafka_admin_connect_write` | write | `create`, `update`, `delete`, `restart`, `pause`, `resume` |
9+
<!-- generated:operations:end -->
10+
311
**Claude connector safety:** Actual MCP tools are split into `kafka_admin_connect_read` and `kafka_admin_connect_write`. The read tool is read-only and only exposes read operations/parameters. The write tool is destructive and is not registered in read-only mode.
412

513
### `kafka_admin_connect_read`

docs/tools/kafka_admin_groups.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
#### kafka-admin-groups
22

3+
4+
<!-- generated:operations:start -->
5+
| Tool | Mode | Operations |
6+
|---|---|---|
7+
| `kafka_admin_groups_read` | read | `list`, `describe`, `offsets` |
8+
| `kafka_admin_groups_write` | write | `remove-members`, `delete-offset`, `set-offset` |
9+
<!-- generated:operations:end -->
10+
311
**Claude connector safety:** Actual MCP tools are split into `kafka_admin_groups_read` and `kafka_admin_groups_write`. The read tool is read-only and only exposes read operations/parameters. The write tool is destructive and is not registered in read-only mode.
412

513
### `kafka_admin_groups_read`

docs/tools/kafka_admin_schema_registry.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
#### kafka-admin-schema-registry
22

3+
4+
<!-- generated:operations:start -->
5+
| Tool | Mode | Operations |
6+
|---|---|---|
7+
| `kafka_admin_sr_read` | read | `list`, `get` |
8+
| `kafka_admin_sr_write` | write | `set`, `create`, `delete` |
9+
<!-- generated:operations:end -->
10+
311
**Claude connector safety:** Actual MCP tools are split into `kafka_admin_sr_read` and `kafka_admin_sr_write`. The read tool is read-only and only exposes read operations/parameters. The write tool is destructive and is not registered in read-only mode.
412

513
### `kafka_admin_sr_read`

docs/tools/kafka_admin_topics.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
#### kafka-admin-topics
22

3+
4+
<!-- generated:operations:start -->
5+
| Tool | Mode | Operations |
6+
|---|---|---|
7+
| `kafka_admin_topics_read` | read | `list`, `get`, `metadata` |
8+
| `kafka_admin_topics_write` | write | `create`, `delete` |
9+
<!-- generated:operations:end -->
10+
311
**Claude connector safety:** Actual MCP tools are split into `kafka_admin_topics_read` and `kafka_admin_topics_write`. The read tool is read-only and only exposes read operations/parameters. The write tool is destructive and is not registered in read-only mode.
412

513
### `kafka_admin_topics_read`

docs/tools/pulsar_admin_brokers.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
#### pulsar_admin_brokers_read / pulsar_admin_brokers_write
22

3+
4+
<!-- generated:operations:start -->
5+
| Tool | Mode | Operations |
6+
|---|---|---|
7+
| `pulsar_admin_brokers_read` | read | `list`, `get` |
8+
| `pulsar_admin_brokers_write` | write | `update`, `delete` |
9+
<!-- generated:operations:end -->
10+
311
**Claude connector safety:** Actual MCP tools are split into `pulsar_admin_brokers_read` and `pulsar_admin_brokers_write`. The read tool is read-only and only exposes read operations/parameters. The write tool is destructive and is not registered in read-only mode.
412

513
### `pulsar_admin_brokers_read`

docs/tools/pulsar_admin_clusters.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
#### pulsar_admin_cluster_read / pulsar_admin_cluster_write
22

3+
4+
<!-- generated:operations:start -->
5+
| Tool | Mode | Operations |
6+
|---|---|---|
7+
| `pulsar_admin_cluster_read` | read | `list`, `get` |
8+
| `pulsar_admin_cluster_write` | write | `create`, `update`, `delete` |
9+
<!-- generated:operations:end -->
10+
311
**Claude connector safety:** Actual MCP tools are split into `pulsar_admin_cluster_read` and `pulsar_admin_cluster_write`. The read tool is read-only and only exposes read operations/parameters. The write tool is destructive and is not registered in read-only mode.
412

513
### `pulsar_admin_cluster_read`

docs/tools/pulsar_admin_functions.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
#### pulsar_admin_functions_read / pulsar_admin_functions_write
22

3+
4+
<!-- generated:operations:start -->
5+
| Tool | Mode | Operations |
6+
|---|---|---|
7+
| `pulsar_admin_functions_read` | read | `list`, `get`, `status`, `stats`, `querystate`, `download` |
8+
| `pulsar_admin_functions_write` | write | `create`, `update`, `delete`, `start`, `stop`, `restart`, `putstate`, `trigger`, `upload` |
9+
<!-- generated:operations:end -->
10+
311
**Claude connector safety:** Actual MCP tools are split into `pulsar_admin_functions_read` and `pulsar_admin_functions_write`. The read tool is read-only and only exposes read operations/parameters. The write tool is destructive and is not registered in read-only mode.
412

513
Pulsar Functions are lightweight compute processes that consume messages from Pulsar topics, apply user-defined logic, and optionally produce results to another topic.

docs/tools/pulsar_admin_namespaces.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
#### pulsar_admin_namespace_read / pulsar_admin_namespace_write
22

3+
4+
<!-- generated:operations:start -->
5+
| Tool | Mode | Operations |
6+
|---|---|---|
7+
| `pulsar_admin_namespace_read` | read | `list`, `get_topics` |
8+
| `pulsar_admin_namespace_write` | write | `create`, `delete`, `clear_backlog`, `unsubscribe`, `unload`, `split_bundle` |
9+
<!-- generated:operations:end -->
10+
311
**Claude connector safety:** Actual MCP tools are split into `pulsar_admin_namespace_read` and `pulsar_admin_namespace_write`. The read tool is read-only and only exposes read operations/parameters. The write tool is destructive and is not registered in read-only mode.
412

513
### `pulsar_admin_namespace_read`

docs/tools/pulsar_admin_nsisolationpolicy.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
#### pulsar_admin_nsisolationpolicy_read / pulsar_admin_nsisolationpolicy_write
22

3+
4+
<!-- generated:operations:start -->
5+
| Tool | Mode | Operations |
6+
|---|---|---|
7+
| `pulsar_admin_nsisolationpolicy_read` | read | `get`, `list` |
8+
| `pulsar_admin_nsisolationpolicy_write` | write | `set`, `delete` |
9+
<!-- generated:operations:end -->
10+
311
**Claude connector safety:** Actual MCP tools are split into `pulsar_admin_nsisolationpolicy_read` and `pulsar_admin_nsisolationpolicy_write`. The read tool is read-only and only exposes read operations/parameters. The write tool is destructive and is not registered in read-only mode.
412

513
Namespace isolation policies control which brokers specific namespaces can use.

0 commit comments

Comments
 (0)