diff --git a/cmd/rune-mcp/main.go b/cmd/rune-mcp/main.go index 50b61fc4..62df6218 100644 --- a/cmd/rune-mcp/main.go +++ b/cmd/rune-mcp/main.go @@ -6,9 +6,10 @@ // Tools: 8 MCP tools (capture, recall, batch_capture, capture_history, // delete_capture, vault_status, diagnostics, reload_pipelines). // -// Phase A (current): MCP handshake + tools/list only. All 8 handlers return -// "not yet implemented" CallToolResult. RunBootLoop · Vault · envector · -// embedder are not wired. Phase 4-5 brings real adapters + service logic. +// Wiring: Deps holds a State manager + 3 services. Adapter clients (vault / +// envector / embedder) are populated on the services by the boot loop after +// Vault returns the bundle. Until boot completes, write tools fail with +// PIPELINE_NOT_READY through CheckState; read-only tools work degraded. // // Python reference: mcp/server/server.py (2002 LoC) package main @@ -23,7 +24,9 @@ import ( sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" + "github.com/envector/rune-go/internal/lifecycle" "github.com/envector/rune-go/internal/mcp" + "github.com/envector/rune-go/internal/service" ) // version is the rune-mcp protocol version surfaced in MCP `initialize`. @@ -46,8 +49,8 @@ func main() { } }() - // Phase A: empty Deps. RunBootLoop / config.Load / adapter wiring deferred. - deps := &mcp.Deps{} + deps := buildDeps() + go lifecycle.RunBootLoop(ctx, deps.State) srv := sdkmcp.NewServer(&sdkmcp.Implementation{ Name: "rune-mcp", @@ -72,3 +75,28 @@ func main() { func isNormalShutdown(err error) bool { return err == nil || errors.Is(err, context.Canceled) } + +// buildDeps wires the state manager + 3 services so that handler dispatch can +// proceed immediately. Adapter clients (vault.Client, embedder.Client, +// envector.Client) and DEK/key state are populated by RunBootLoop once Vault +// returns the bundle — until then, the services see nil adapters and write +// tools are state-gated to PIPELINE_NOT_READY. +// +// State is shared across services so a single Manager.SetState transition +// updates the gate from every code path uniformly. +func buildDeps() *mcp.Deps { + mgr := lifecycle.NewManager() + + cap := service.NewCaptureService() + cap.State = mgr + + life := service.NewLifecycleService() + life.State = mgr + + return &mcp.Deps{ + State: mgr, + Capture: cap, + Recall: service.NewRecallService(), + Lifecycle: life, + } +} diff --git a/docs/v04/notes/audit-mcp-tools-impl.md b/docs/v04/notes/audit-mcp-tools-impl.md new file mode 100644 index 00000000..f01edc8b --- /dev/null +++ b/docs/v04/notes/audit-mcp-tools-impl.md @@ -0,0 +1,196 @@ +# mcp-tools-impl Python Parity Audit (Tasks 1 + 2) + +> Run on `couragehong/feat/mcp-tools-impl` HEAD `c52b814` +> against `mcp/server/server.py` (~2002 LoC). +> Spec: `docs/v04/spec/components/rune-mcp.md`, +> `docs/v04/overview/architecture.md §Scope`. + +## Verdict + +**Pass on the agent-delegated path that v0.4 targets**. Tool catalog, +state-gating, validation, and error-shape match. Two intentional Go +divergences: `rune_` prefix on tool names (matches docs/spec, not Python), +and atomic state machine instead of asyncio Event for readiness gating. + +## Tool catalog parity + +| Python tool name | Go tool name | Gated? | +|---|---|---| +| `capture` (server.py:L688) | `rune_capture` | ✅ both gated | +| `batch_capture` (L810) | `rune_batch_capture` | ✅ both gated | +| `recall` (L901) | `rune_recall` | ✅ both gated | +| `delete_capture` (L1115) | `rune_delete_capture` | ✅ both gated | +| `capture_history` (L1093) | `rune_capture_history` | ✅ both bypass | +| `vault_status` (L492) | `rune_vault_status` | ✅ both bypass | +| `diagnostics` (L532) | `rune_diagnostics` | ✅ both bypass | +| `reload_pipelines` (L1038) | `rune_reload_pipelines` | ✅ both bypass | + +**Naming divergence**: Python uses bare names (`capture`, `recall`, …); Go +uses `rune_` prefix. The spec `docs/v04/spec/components/rune-mcp.md` L124 +shows the Go form (`rune_capture`) in the SDK code sample but section +headers (L161 `tool_capture`, L173 `tool_recall`) cite the Python name. +**Decision**: keep `rune_` prefix (better tool namespacing in MCP — many +servers may expose a `capture` tool, only one a `rune_capture`). Document +this in spec as intentional. Track as a doc consistency followup. + +## Behavior parity + +### State gate (Python `_ensure_pipelines` ↔ Go `CheckState`) + +| Aspect | Python (server.py:L1503-1518) | Go (state.go:CheckState) | +|---|---|---| +| Mechanism | `Event.wait(timeout=120s)` blocks until `_pipelines_ready` set | atomic state machine via `lifecycle.Manager.Current()` | +| State branches | (init not done) / (init failed) | starting / waiting_for_vault / dormant / active | +| Recovery hints | static ("embedding model may still be downloading", "Run /rune:activate") | state-specific ("Wait 1-2 seconds", "Run /rune:vault_status", "Run /rune:activate") | +| Error code | `PipelineNotReadyError` → `code="PIPELINE_NOT_READY"` | `domain.ErrPipelineNotReady` (same code string) | + +Python has finer-grained timeout logic (block for up to 120 s); Go just +returns immediately with current state. **Different paradigms, same +contract from the agent's perspective** — both surface `PIPELINE_NOT_READY` +when the pipeline isn't usable. Go is simpler and doesn't tie up a +goroutine waiting for boot. + +### Input validation + +| Tool | Python check | Go check | +|---|---|---| +| capture | `parse_llm_json(extracted)` returns None → "Invalid extracted JSON" (L1240-1242) | `req.Extracted == nil` → ErrInvalidInput (state.go ValidateCaptureRequest) | +| capture | text trim — implicit (Python doesn't enforce; rejects later) | `strings.TrimSpace(req.Text) == ""` → ErrInvalidInput (Go is **stricter** — fail-fast) | +| capture | tier2.capture=false rejection (L1246-1251) | done in `domain.ParseExtractionFromAgent` (extraction.go) | +| recall | topk > 10 → InvalidInputError (L930-931) | `args.TopK > 10` → ErrInvalidInput (state.go ValidateRecallArgs) | +| recall | (topk default 5 in signature) | `args.TopK == 0 → 5` (state.go) | +| recall | (no empty-query check) | `strings.TrimSpace(args.Query) == ""` → ErrInvalidInput (Go **stricter**) | + +Two Go-stricter checks (capture text empty, recall query empty) — both +defensive additions, match D24 spec early-reject intent. + +### Error response shape + +Python `make_error` (server.py-imported from `mcp/server/errors.py`): +```python +{ + "ok": false, + "error": { + "code": "VAULT_CONNECTION_ERROR", + "message": "...", + "retryable": true, + "recovery_hint": "..." # optional + } +} +``` + +Go `domain.MakeError` (errors.go:L48-72) produces the **identical** map +shape, then `internal/mcp/result.go errorResult()` JSON-marshals it into +`TextContent` with `IsError: true`. + +✅ Wire-shape bit-identical. + +### Agent-delegated mode (v0.4 architecture-level decision) + +Python `tool_capture` has TWO modes (server.py:L737-775): + +```python +# ===== PRIMARY: Agent-delegated mode ===== +if extracted is not None: + return await self._capture_single(...) + +# ===== FALLBACK: Legacy 3-tier pipeline (requires API keys) ===== +# Retained for backward compatibility. +``` + +v0.4 architecture (`docs/v04/overview/architecture.md §Scope`) commits to +**agent-delegated only** (D14/D21/D28). Go drops the legacy fallback — +`extracted=None` is treated as a hard `ExtractionMissing` error rather +than triggering server-side LLM extraction. + +✅ This is the intentional v0.4 narrowing of scope. + +## Specific Go-only improvements + +1. **Type-safe handler signatures**. Python tool decorator forwards args + through `Annotated[T, Field(description=...)]` Pydantic; Go uses + `ToolHandlerFor[In, Out]` with concrete struct types. Compile-time + schema inference vs runtime introspection. +2. **Compile-time tool name validation** (`mustAdd` panic-guard). The + official Go SDK only logs on bad names; Python's FastMCP enforces. + `mustAdd` brings Go up to Python's safety level. +3. **Nil-safe `CheckState`**. `Deps.State == nil` returns + `PIPELINE_NOT_READY` with hint instead of panicking — useful for tests + and during boot before Manager is constructed. +4. **State-specific recovery hints**. Python uses a single static hint + ("Run /rune:activate"); Go branches by state (starting / waiting_for_vault + / dormant) for more actionable messages. + +## Acceptable divergences (⚠️) + +1. **Tool name prefix**. `rune_*` (Go) vs bare names (Python). See above. +2. **Capture path 1-mode**. Drops Python's legacy 3-tier fallback. Per D14. +3. **Validation strictness**. Go rejects empty text/query early; Python lets + them flow further before failing. Strict-fail is friendlier. + +## Open gaps (⚠️ should follow up) + +### 1. **State.go validation does NOT cover phases / title truncation / confidence clamp** + +Python (server.py:L1271-1280, L1252-1261) does these inline in `_capture_single`: +- `phases_data[:7]` (max 7 phases) +- `phase_title[:60]` (max 60 chars) +- `confidence = max(0.0, min(1.0, ...))` clamp + +Go has helpers (`TruncateTitle`, `ClampConfidence` in state.go) but they +are NOT called from any handler — they are referenced in +`docs/v04/spec/components/rune-mcp.md §state.go` but actual call sites are +in `internal/domain/extraction.go` `ParseExtractionFromAgent`. Need to +verify the parser does these consistently. (Out of mcp-tools-impl scope — +domain/extraction.go is in #96.) + +### 2. **`_maybe_reload_for_auto_provider`** is dropped (D31, archived) + +Python L484-488 optionally reloads pipelines based on MCP clientInfo header. +v0.4 explicitly drops this (D31). Go has no equivalent. ✅ Intentional. + +### 3. **`request_id` propagation** + +Python adds context-bound request IDs via the FastMCP context arg. Go's +handlers don't yet thread `request_id` through. Spec promises slog + +request_id for observability (`rune-mcp.md §Observability`). Track as a +follow-up. + +### 4. **Boot loop is a stub** + +`internal/lifecycle/boot.go` `RunBootLoop` body is `_ = m`. Until it dials +Vault and populates services with adapter clients, write tools always fail +with `PIPELINE_NOT_READY`. End-to-end behavior requires: +- vault-grpc PR (this is its #98 partner) +- runed-grpc PR (#99 partner) +- Boot loop body (separate followup) + +**This is the largest gap** — flagged for production-readiness audit (Task #5). + +### 5. **`buildDeps` does NOT wire State into `RecallService`** + +`RecallService` does not have a `State` field per spec +(`internal/service/recall.go`). It uses `s.Vault` etc. but doesn't gate on +state internally — relies on the handler's CheckState. ✅ Working as +intended; just noting. + +### 6. **`CaptureService.Now` clock injection** + +Python uses `datetime.now(timezone.utc)` directly. Go injects `Now func() +time.Time` for testability. ✅ Go improvement. + +## Test coverage status + +`go test ./internal/mcp/` covers: +- `TestRegister_All8ToolsListed` — catalog presence + alphabetical order +- `TestRegister_SchemasInferred` — input/output schema inference per tool +- `TestRegister_WriteToolsGated` — 4 write tools surface PIPELINE_NOT_READY in starting state +- `TestIsValidToolName`, `TestMustAdd_PanicsOnInvalidName` — name validation +- `TestRegister_AllHardcodedNamesValid` — Register doesn't panic on any name + +Gaps: +- read-only tool happy-paths (vault_status nil-Vault, diagnostics, capture_history) +- error result wrapping fidelity (errorResult preserves RuneError code/retryable/hint) +- okResult JSON-marshal correctness + +Track these as Task #8. diff --git a/docs/v04/notes/audit-production-readiness.md b/docs/v04/notes/audit-production-readiness.md new file mode 100644 index 00000000..deea6ed2 --- /dev/null +++ b/docs/v04/notes/audit-production-readiness.md @@ -0,0 +1,199 @@ +# Production Readiness Audit (Tasks 1-4) + +> Cross-cuts the three task PRs (`couragehong/feat/{vault-grpc, runed-grpc, +> mcp-tools-impl}`) plus integration base `wip/go-impl-runtime` (which folds +> in #93/#95/#96). Question answered: **"Can rune-mcp boot, accept a real +> capture, and complete it end-to-end with this code?"**. + +## Verdict + +**No, not yet — one P0 blocker, three P1 gaps**. The 4 task PRs deliver +adapter / handler skeletons that compile, register, and respond to a state +gate. They do **not** yet connect to live Vault / runed at boot; one more +focused PR ("boot loop body + adapter wiring + config loader") is the +critical missing piece. Below is the priority-ordered gap list with +concrete file/line targets. + +## What works today + +With all 3 task branches + #93/#95/#96 merged locally: + +- `go build ./...` ✅ +- `go test ./...` ✅ (5 packages have tests) +- `rune-mcp` binary boots, registers 8 tools, accepts MCP `initialize` / + `tools/list` / `tools/call` +- Read-only tools partially functional in `StateStarting`: + - `rune_vault_status` returns `mode: "standard (no Vault)"` since + `LifecycleService.Vault == nil` + - `rune_diagnostics` returns env section (Go version, OS) but Vault / + embedder / envector sections empty + - `rune_capture_history` reads `~/.rune/capture_log.jsonl` if present +- Write tools surface `PIPELINE_NOT_READY` (correct) — will stay that way + until boot loop transitions State to Active + +## P0 — boot loop body (BLOCKER for end-to-end) + +**File**: `internal/lifecycle/boot.go:RunBootLoop` +**Current**: `_ = m` — no-op stub. Daemon stays in StateStarting forever. + +**Required**: +1. Load config (`config.Load("~/.rune/config.json")`) +2. Construct `vault.Client` (`vault.NewClient(endpoint, token, opts)`) +3. Loop: call `vault.GetPublicKey(ctx)` with backoff + `[1, 2, 5, 15, 30, 60]s` (already in `BootBackoffs`) + - Success → write `EncKey.json` / `EvalKey.json` to + `~/.rune/keys//`, construct `embedder.New(sockPath)`, + construct `envector.NewClient(...)`, populate the 3 service structs' + adapter fields, `m.SetState(Active)`, return + - Fail → `m.SetState(WaitingForVault)`, log, sleep, retry +4. Run forever in background (re-enter on Vault death) + +**Why P0**: Without this, write tools always fail; rune-mcp can't actually +do its job. + +**Estimated effort**: 1 dedicated PR, ~150 LoC. Depends on: +- vault-grpc (Task 4) — provides `vault.NewClient` +- runed-grpc (Task 3) — provides `embedder.New` +- envector adapter from #95 — provides `envector.NewClient` + +Cleanest sequencing: this PR lands AFTER vault-grpc + runed-grpc merge +upstream and #95 closes review. + +## P1 — config / env wiring + +### 1.1 `RUNEVAULT_GRPC_TARGET` env override (vault-grpc gap) + +Python `vault_client.py:L108-110` honors this env var as the gRPC target +override (escape hatch for ops/IR). Go `vault.NewClient` doesn't check +env; caller must pass the resolved endpoint. + +**File**: `internal/adapters/config/loader.go` or new `cmd/rune-mcp/main.go` +helper. Resolution priority should be: + +``` +RUNEVAULT_GRPC_TARGET (env, if set, full host:port override) +> RUNEVAULT_ENDPOINT (env) +> config.Vault.Endpoint (config.json) +``` + +**Effort**: ~20 LoC. + +### 1.2 Embedder socket path resolution (runed-grpc gap) + +`embedder.New(sockPath)` takes a resolved path; comment promises priority +chain `RUNE_EMBEDDER_SOCKET > config.embedder.socket_path > default +~/.runed/embedding.sock`. Not implemented anywhere. + +**File**: `cmd/rune-mcp/main.go buildDeps` or +`internal/adapters/embedder/socket.go`. ~15 LoC. + +### 1.3 FHE key disk persistence on bundle receive + +Spec `vault.md §부팅 시퀀스` step 3: +```go +saveKeysToDisk(bundle.EncKey, bundle.EvalKey) // ~/.rune/keys// +``` + +Not implemented. Need `internal/adapters/keystore/` (or in lifecycle/boot.go) +to persist `EncKey.json`, `EvalKey.json` at `0600` to +`~/.rune/keys//` so `envector.OpenKeysFromFile` can read +them. + +**Effort**: ~40 LoC. + +## P2 — observability + safety + +### 2.1 `request_id` propagation (mcp-tools-impl gap) + +Spec `rune-mcp.md §Observability` promises every tool call gets a UUID +threaded through context for slog. Go `internal/obs/request_id.go` exists +(per #95) but handlers don't push it onto ctx. + +**File**: `internal/mcp/handlers.go` — wrap each handler's `ctx` with +`obs.WithRequestID(ctx, obs.NewRequestID())` before dispatch. Adapters +already have `obs.RequestID(ctx)` available. + +**Effort**: ~10 LoC. Worth doing in mcp-tools-impl follow-up. + +### 2.2 `vault.HealthCheck` Tier 2 fallback hookup (vault-grpc gap) + +`vault.HealthFallback` exists as a standalone function but no caller. The +spec intends `LifecycleService.Diagnostics` / `VaultStatus` to invoke it +when Tier 1 fails to enrich the diagnostic message ("gRPC port down but +HTTP /health OK"). + +**File**: `internal/service/lifecycle.go collectVault` — on +`HealthCheck() == false`, also try `vault.HealthFallback(ctx, endpoint)` +and report Tier 2 result distinctly. + +**Effort**: ~15 LoC. + +### 2.3 runed `vector_dim` verification (runed-grpc gap) + +Spec promises Embed/EmbedBatch verify `len(vec) == Info.VectorDim`. Not +done. Silent dim-mismatch could corrupt index. + +**File**: `internal/adapters/embedder/client.go embedBatchOnce` and +`EmbedSingle`. + +**Effort**: ~10 LoC. + +### 2.4 runed typed error sentinels + +Match `vault/errors.go`'s pattern: 5 typed error sentinels + +`MapGRPCError`. Currently `embedder` returns raw `fmt.Errorf`-wrapped gRPC +errors; service layer can introspect via `status.FromError` but loses the +adapter-namespaced retryable/code metadata. + +**File**: `internal/adapters/embedder/errors.go` (new). ~60 LoC. + +## P3 — nice-to-haves + +- **Doc consistency** for `rune_*` tool name prefix. Current spec + references both forms; pick one and update all callsites uniformly. +- **integration tag tests** for both gRPC adapters (`//go:build + integration`). Real Vault + real runed daemon. CI with secret service. +- **Dormant reason persistence**. Spec Q9 — when Vault permanently fails, + persist `dormant_reason: "vault_unreachable"` to config.json so next + boot starts dormant. + +## Gating criteria for "production-ready end-to-end" + +When all P0 + P1 items land, an end-to-end test should be: + +```bash +# prereqs +cd ../rune-admin/vault && go run ./cmd start --config testdata/dev.yaml & +cd ../runed && go run ./cmd/runed serve & + +# rune side +cat > ~/.rune/config.json < 0 { + if tc0, ok := res.Content[0].(*sdkmcp.TextContent); ok { + body = tc0.Text + } + } + t.Fatalf("IsError: got true, want false (read-only tools bypass gate). body=%s", body) } if len(res.Content) == 0 { t.Fatalf("Content: empty") @@ -147,12 +246,71 @@ func TestRegister_StubReturnsIsError(t *testing.T) { if !ok { t.Fatalf("Content[0]: got %T, want *TextContent", res.Content[0]) } - if !strings.Contains(tc0.Text, "not yet implemented") { - t.Errorf("Content[0].Text: %q does not contain stub marker", tc0.Text) + for _, want := range tc.mustContain { + if !strings.Contains(tc0.Text, want) { + t.Errorf("Content[0].Text missing %q in: %s", want, tc0.Text) + } } - if !strings.Contains(tc0.Text, tc.name) { - t.Errorf("Content[0].Text: %q does not name the tool", tc0.Text) + for _, deny := range tc.mustNotContain { + if strings.Contains(tc0.Text, deny) { + t.Errorf("Content[0].Text contains %q (should not): %s", deny, tc0.Text) + } } }) } } + +// TestRegister_ErrorResultPreservesRuneError — verifies the {ok,error{code, +// message,retryable,recovery_hint}} shape is carried bit-identical in the +// TextContent JSON when handlers fail. Uses the state-gate path which is +// guaranteed to surface PIPELINE_NOT_READY in StateStarting. +func TestRegister_ErrorResultPreservesRuneError(t *testing.T) { + cs := newSession(t) + + res, err := cs.CallTool(t.Context(), &sdkmcp.CallToolParams{ + Name: "rune_capture", + Arguments: map[string]any{"text": "hi", "source": "test", "extracted": map[string]any{}}, + }) + if err != nil { + t.Fatalf("CallTool transport: %v", err) + } + if !res.IsError { + t.Fatal("IsError: want true for state-gated tool") + } + if len(res.Content) == 0 { + t.Fatal("Content: empty") + } + tc0, ok := res.Content[0].(*sdkmcp.TextContent) + if !ok { + t.Fatalf("Content[0]: %T not *TextContent", res.Content[0]) + } + + var body struct { + OK bool `json:"ok"` + Error struct { + Code string `json:"code"` + Message string `json:"message"` + Retryable bool `json:"retryable"` + RecoveryHint string `json:"recovery_hint"` + } `json:"error"` + } + if err := json.Unmarshal([]byte(tc0.Text), &body); err != nil { + t.Fatalf("TextContent not parseable as MakeError JSON: %v\nbody=%s", err, tc0.Text) + } + + if body.OK { + t.Errorf("ok: got true, want false") + } + if body.Error.Code != "PIPELINE_NOT_READY" { + t.Errorf("error.code: got %q, want PIPELINE_NOT_READY", body.Error.Code) + } + if body.Error.Retryable { + t.Errorf("error.retryable: got true, want false (PIPELINE_NOT_READY is not retryable)") + } + if body.Error.RecoveryHint == "" { + t.Error("error.recovery_hint: empty (CheckState should populate state-specific hint)") + } + if !strings.Contains(body.Error.RecoveryHint, "starting") { + t.Errorf("error.recovery_hint: %q does not mention starting state", body.Error.RecoveryHint) + } +} diff --git a/internal/mcp/result.go b/internal/mcp/result.go new file mode 100644 index 00000000..06bb7452 --- /dev/null +++ b/internal/mcp/result.go @@ -0,0 +1,42 @@ +package mcp + +import ( + "encoding/json" + + sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" + + "github.com/envector/rune-go/internal/domain" +) + +// errorResult shapes a tool failure into a CallToolResult per the rune-mcp +// error contract (spec/components/rune-mcp.md §에러 처리). +// +// The body is JSON-marshalled domain.MakeError output so the agent receives +// {"ok":false,"error":{"code","message","retryable","recovery_hint"}}. We mark +// IsError=true so the SDK forwards the error semantics to the caller. +func errorResult(err error) *sdkmcp.CallToolResult { + body, _ := json.Marshal(domain.MakeError(err)) + return &sdkmcp.CallToolResult{ + IsError: true, + Content: []sdkmcp.Content{&sdkmcp.TextContent{Text: string(body)}}, + } +} + +// okResult builds a success CallToolResult whose TextContent is the +// JSON-encoded result payload. Out (typed) is also returned by the handler +// so the SDK populates StructuredContent. +// +// Failure to marshal payload (rare — only structurally bad output types) is +// reported as INTERNAL_ERROR rather than panicking. +func okResult(payload any) *sdkmcp.CallToolResult { + body, err := json.Marshal(payload) + if err != nil { + return errorResult(&domain.RuneError{ + Code: domain.CodeInternal, + Message: "failed to marshal result: " + err.Error(), + }) + } + return &sdkmcp.CallToolResult{ + Content: []sdkmcp.Content{&sdkmcp.TextContent{Text: string(body)}}, + } +} diff --git a/internal/mcp/state.go b/internal/mcp/state.go index 08d3da11..80c47bee 100644 --- a/internal/mcp/state.go +++ b/internal/mcp/state.go @@ -18,6 +18,9 @@ import ( // - dormant(vault) → "Check config.vault.endpoint" // - dormant(envector) → "Check network · API key" func CheckState(m *lifecycle.Manager) error { + if m == nil { + return withHint(domain.ErrPipelineNotReady, "rune-mcp boot has not been wired (Deps.State == nil).") + } switch m.Current() { case lifecycle.StateActive: return nil diff --git a/internal/mcp/tools.go b/internal/mcp/tools.go index 605e063d..144996f7 100644 --- a/internal/mcp/tools.go +++ b/internal/mcp/tools.go @@ -16,7 +16,6 @@ package mcp import ( - "context" "fmt" sdkmcp "github.com/modelcontextprotocol/go-sdk/mcp" @@ -24,27 +23,23 @@ import ( "github.com/envector/rune-go/internal/adapters/embedder" "github.com/envector/rune-go/internal/adapters/envector" "github.com/envector/rune-go/internal/adapters/vault" - "github.com/envector/rune-go/internal/domain" "github.com/envector/rune-go/internal/lifecycle" "github.com/envector/rune-go/internal/service" ) // Deps — injected into all 8 MCP handlers. // -// Phase A: empty struct. Adapter clients · state machine · config will be -// added as Phase 4 (adapters) and Phase 5 (service orchestration) land. -// stubHandler already takes deps as an argument, so Phase 5 will only need -// to swap the closure body, not the signature. -// -// Future fields (commented as a contract sketch — to be activated as the -// owning adapter PR lands): -// +// State + 3 services drive request handling. cmd/rune-mcp/main.go constructs +// Deps after the boot loop has populated adapter clients on the services. +// Until boot completes, write tools fail with PIPELINE_NOT_READY through +// CheckState; read-only tools (vault_status, diagnostics, capture_history) +// can run pre-active for diagnostics. type Deps struct { - Vault vault.Client - Envector envector.Client - Embedder embedder.Client - State *lifecycle.Manager - + Vault vault.Client + Envector envector.Client + Embedder embedder.Client + State *lifecycle.Manager + Capture *service.CaptureService Recall *service.RecallService Lifecycle *service.LifecycleService @@ -52,23 +47,41 @@ type Deps struct { func (d *Deps) InjectVault(client vault.Client) { d.Vault = client - if d.Capture != nil { d.Capture.Vault = client } - if d.Recall != nil { d.Recall.Vault = client } - if d.Lifecycle != nil { d.Lifecycle.Vault = client } + if d.Capture != nil { + d.Capture.Vault = client + } + if d.Recall != nil { + d.Recall.Vault = client + } + if d.Lifecycle != nil { + d.Lifecycle.Vault = client + } } func (d *Deps) InjectEmbedder(client embedder.Client) { d.Embedder = client - if d.Capture != nil { d.Capture.Embedder = client } - if d.Recall != nil { d.Recall.Embedder = client } - if d.Lifecycle != nil { d.Lifecycle.Embedder = client } + if d.Capture != nil { + d.Capture.Embedder = client + } + if d.Recall != nil { + d.Recall.Embedder = client + } + if d.Lifecycle != nil { + d.Lifecycle.Embedder = client + } } func (d *Deps) InjectEnvector(client envector.Client) { d.Envector = client - if d.Capture != nil { d.Capture.Envector = client } - if d.Recall != nil { d.Recall.Envector = client } - if d.Lifecycle != nil { d.Lifecycle.Envector = client } + if d.Capture != nil { + d.Capture.Envector = client + } + if d.Recall != nil { + d.Recall.Envector = client + } + if d.Lifecycle != nil { + d.Lifecycle.Envector = client + } } // ApplyVaultBundle propagates per-bundle metadata (AgentID / AgentDEK / @@ -109,8 +122,8 @@ type emptyArgs struct{} // // Failure modes that Register surfaces as a startup error (via panic + // recover): -// 1. mustAddTool name validation (SDK's validateToolName has a log-only -// branch — server.go:238-241 — that we bypass by panicking up-front). +// 1. mustAdd name validation (SDK's validateToolName has a log-only branch — +// server.go:238-241 — that we bypass by panicking up-front). // 2. SDK schema-inference panic (toolForErr). // 3. SDK schema-shape panic (Server.AddTool). // @@ -123,52 +136,52 @@ func Register(srv *sdkmcp.Server, deps *Deps) (err error) { } }() - // Write tools (state gate applies in Phase 5). - mustAddTool[domain.CaptureRequest, domain.CaptureResponse](srv, deps, - "rune_capture", - "Capture a decision record (agent-delegated extraction required).") - mustAddTool[service.BatchCaptureArgs, service.BatchCaptureResult](srv, deps, - "rune_batch_capture", - "Capture a batch of decision records (e.g. session-end sweep).") - mustAddTool[domain.RecallArgs, domain.RecallResult](srv, deps, - "rune_recall", - "Query organizational memory by natural-language question.") - mustAddTool[service.DeleteCaptureArgs, service.DeleteCaptureResult](srv, deps, - "rune_delete_capture", - "Soft-delete a record by ID (sets status=reverted, re-inserts).") - - // Read / diagnostic tools (state gate bypass). - mustAddTool[service.CaptureHistoryArgs, service.CaptureHistoryResult](srv, deps, - "rune_capture_history", - "List recent captures from local capture_log.jsonl (read-only).") - mustAddTool[emptyArgs, service.VaultStatusResult](srv, deps, - "rune_vault_status", - "Probe Vault connectivity and report secure-search mode.") - mustAddTool[emptyArgs, service.DiagnosticsResult](srv, deps, - "rune_diagnostics", - "Collect a 7-section health snapshot (env / state / vault / keys / pipelines / embedding / envector).") - mustAddTool[emptyArgs, service.ReloadPipelinesResult](srv, deps, - "rune_reload_pipelines", - "Re-initialize Vault + envector pipelines (BOOT replay) with envector warmup.") + // Write tools — state-gated. + mustAdd(srv, "rune_capture", + "Capture a decision record (agent-delegated extraction required).", + handleCapture(deps)) + mustAdd(srv, "rune_batch_capture", + "Capture a batch of decision records (e.g. session-end sweep).", + handleBatchCapture(deps)) + mustAdd(srv, "rune_recall", + "Query organizational memory by natural-language question.", + handleRecall(deps)) + mustAdd(srv, "rune_delete_capture", + "Soft-delete a record by ID (sets status=reverted, re-inserts).", + handleDeleteCapture(deps)) + + // Read / diagnostic tools — bypass state gate. + mustAdd(srv, "rune_capture_history", + "List recent captures from local capture_log.jsonl (read-only).", + handleCaptureHistory(deps)) + mustAdd(srv, "rune_vault_status", + "Probe Vault connectivity and report secure-search mode.", + handleVaultStatus(deps)) + mustAdd(srv, "rune_diagnostics", + "Collect a 7-section health snapshot (env / state / vault / keys / pipelines / embedding / envector).", + handleDiagnostics(deps)) + mustAdd(srv, "rune_reload_pipelines", + "Re-initialize Vault + envector pipelines (BOOT replay) with envector warmup.", + handleReloadPipelines(deps)) return nil } -// mustAddTool wraps sdkmcp.AddTool with up-front name validation. +// mustAdd wraps sdkmcp.AddTool with up-front name validation. // // The SDK's Server.AddTool only LOGS on invalid tool names // (go-sdk/mcp/server.go:238-241) — it does not panic, so Register's // defer recover() would miss it and the bad-named tool would silently -// register. mustAddTool panics on invalid names, unifying the failure +// register. mustAdd panics on invalid names, unifying the failure // path so recover() catches everything. -func mustAddTool[In, Out any](srv *sdkmcp.Server, deps *Deps, name, description string) { +func mustAdd[In, Out any](srv *sdkmcp.Server, name, description string, h sdkmcp.ToolHandlerFor[In, Out]) { if !isValidToolName(name) { - panic(fmt.Errorf("mustAddTool: invalid tool name %q (allowed: [A-Za-z0-9_-], 1..128 chars)", name)) + panic(fmt.Errorf("mustAdd: invalid tool name %q (allowed: [A-Za-z0-9_-], 1..128 chars)", name)) } sdkmcp.AddTool(srv, &sdkmcp.Tool{ Name: name, Description: description, - }, stubHandler[In, Out](deps, name)) + }, h) } // isValidToolName mirrors the SDK's validateToolName rules @@ -188,28 +201,3 @@ func isValidToolName(name string) bool { return true } -// stubHandler returns a SDK ToolHandlerFor that always responds with a -// not-yet-implemented isError result. Output type is preserved so tools/list -// can still publish the inferred output schema. -// -// deps is captured but unused in Phase A. Phase 5 will dereference it for -// CheckState / service dispatch — the closure shape stays the same. -func stubHandler[In, Out any](deps *Deps, toolName string) sdkmcp.ToolHandlerFor[In, Out] { - _ = deps // captured for Phase 5; intentionally unused now - return func(_ context.Context, _ *sdkmcp.CallToolRequest, _ In) (*sdkmcp.CallToolResult, Out, error) { - var zero Out - return stubResult(toolName), zero, nil - } -} - -// stubResult composes the Phase-A "not implemented" response. -func stubResult(toolName string) *sdkmcp.CallToolResult { - return &sdkmcp.CallToolResult{ - IsError: true, - Content: []sdkmcp.Content{ - &sdkmcp.TextContent{ - Text: toolName + " is not yet implemented (skeleton phase A — MCP handshake + tools/list only).", - }, - }, - } -} diff --git a/internal/mcp/tools_test.go b/internal/mcp/tools_test.go index 3d097256..aa74e292 100644 --- a/internal/mcp/tools_test.go +++ b/internal/mcp/tools_test.go @@ -4,6 +4,7 @@ package mcp import ( + "context" "strings" "testing" @@ -25,14 +26,17 @@ func TestIsValidToolName(t *testing.T) { } } -func TestMustAddTool_PanicsOnInvalidName(t *testing.T) { +func TestMustAdd_PanicsOnInvalidName(t *testing.T) { defer func() { if r := recover(); r == nil { - t.Error("mustAddTool with invalid name did not panic") + t.Error("mustAdd with invalid name did not panic") } }() srv := sdkmcp.NewServer(&sdkmcp.Implementation{Name: "x", Version: "0"}, nil) - mustAddTool[emptyArgs, emptyArgs](srv, &Deps{}, "rune capture", "test") + noopHandler := func(_ context.Context, _ *sdkmcp.CallToolRequest, _ emptyArgs) (*sdkmcp.CallToolResult, emptyArgs, error) { + return nil, emptyArgs{}, nil + } + mustAdd[emptyArgs, emptyArgs](srv, "rune capture", "test", noopHandler) } func TestRegister_AllHardcodedNamesValid(t *testing.T) {