diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..a1515b3 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,26 @@ +name: CI + +on: + push: + branches: [main] + +jobs: + test: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version-file: go.mod + + - name: Run tests with coverage + run: | + go test -coverprofile=coverage.out ./pkg/mcp/... + sed -i 's|forge.lthn.ai/core/mcp/||g' coverage.out + + - name: Upload to Codecov + uses: codecov/codecov-action@v5 + with: + files: coverage.out + token: ${{ secrets.CODECOV_TOKEN }} diff --git a/CLAUDE.md b/CLAUDE.md index a8383fe..a83a086 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,133 +1,129 @@ # CLAUDE.md -This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. +Guidance for Claude Code and Codex when working with this repository. -## Project overview +## Module -Core MCP is a Model Context Protocol implementation in two halves: a **Go binary** (`core-mcp`) that speaks native MCP over stdio/TCP/Unix, and a **PHP Laravel package** (`lthn/mcp`) that adds an HTTP MCP API with auth, quotas, and analytics. Both halves bridge to each other via REST or WebSocket. +`forge.lthn.ai/core/mcp` — Model Context Protocol server with file operations, tool registration, notification broadcasting, and channel events. -Module: `forge.lthn.ai/core/mcp` | Licence: EUPL-1.2 +Licence: EUPL-1.2 -## Build and test commands - -### Go +## Build & Test ```bash -core build # Build binary (./core-mcp) -go build -o core-mcp ./cmd/core-mcp/ # Alternative without core CLI - -core go test # Run all Go tests -core go test --run TestBridgeToAPI # Run a single test -core go cov # Coverage report -core go cov --open # Open HTML coverage in browser -core go qa # Format + vet + lint + test -core go qa full # Also race detector, vuln scan, security audit -core go fmt # gofmt -core go lint # golangci-lint -core go vet # go vet +go test ./pkg/mcp/... # run all tests +go build ./pkg/mcp/... # verify compilation +go build ./cmd/core-mcp/ # build binary ``` -### PHP (from repo root or `src/php/`) +Or via the Core CLI: ```bash -composer test # Run all PHP tests (Pest) -composer test -- --filter=SqlQueryValidatorTest # Single test -composer lint # Laravel Pint (PSR-12) -./vendor/bin/pint --dirty # Format only changed files +core go test +core go qa # fmt + vet + lint + test ``` -### Running locally +## API Shape -```bash -./core-mcp mcp serve # Stdio transport (Claude Code / IDE) -./core-mcp mcp serve --workspace /path/to/project # Sandbox file ops to directory -MCP_ADDR=127.0.0.1:9100 ./core-mcp mcp serve # TCP transport +Uses `Options{}` struct, not functional options: + +```go +svc, err := mcp.New(mcp.Options{ + WorkspaceRoot: "/path/to/project", + ProcessService: ps, + WSHub: hub, + Subsystems: []mcp.Subsystem{brain, agentic, monitor}, +}) ``` -## Architecture +**Do not use:** `WithWorkspaceRoot`, `WithSubsystem`, `WithProcessService`, `WithWSHub` — these no longer exist. -### Go server (`pkg/mcp/`) +## Notification Broadcasting -`mcp.Service` is the central type, configured via functional options (`mcp.With*`). It owns the MCP server, a sandboxed filesystem `Medium`, optional subsystems, and an ordered `[]ToolRecord` that powers the REST bridge. +```go +// Broadcast to all connected sessions +svc.SendNotificationToAllClients(ctx, "info", "monitor", data) -**Tool registration**: All tools use the generic `addToolRecorded[In, Out]()` function which simultaneously registers the MCP handler, reflects input/output structs into JSON Schemas, and creates a REST handler closure. No per-tool glue code needed. +// Push a named channel event +svc.ChannelSend(ctx, "agent.complete", map[string]any{"repo": "go-io"}) -**Tool groups** (registered in `registerTools()`): -- `files`, `language` — `mcp.go` -- `metrics` — `tools_metrics.go` -- `rag` — `tools_rag.go` -- `process` — `tools_process.go` (requires `WithProcessService`) -- `webview` — `tools_webview.go` -- `ws` — `tools_ws.go` (requires `WithWSHub`) +// Push to a specific session +svc.ChannelSendToSession(ctx, session, "build.failed", data) +``` -**Subsystem interface** (`Subsystem` / `SubsystemWithShutdown`): Pluggable tool groups registered via `WithSubsystem`. Three ship with the repo: -- `tools_ml.go` — ML inference subsystem (generate, score, probe, status, backends) -- `pkg/mcp/ide/` — IDE bridge to Laravel backend over WebSocket (chat, build, dashboard tools) -- `pkg/mcp/brain/` — OpenBrain knowledge store proxy (remember, recall, forget, list) +The `claude/channel` experimental capability is registered automatically. -**Transports**: stdio (default), TCP (`MCP_ADDR` env var), Unix socket (`ServeUnix`). TCP binds `127.0.0.1` by default; `0.0.0.0` emits a security warning. +## Tool Groups -**REST bridge**: `BridgeToAPI` maps each `ToolRecord` to a `POST` endpoint via `api.ToolBridge`. 10 MB body limit. +| File | Group | Tools | +|------|-------|-------| +| `mcp.go` | files, language | file_read, file_write, file_delete, file_rename, file_exists, file_edit, dir_list, dir_create, lang_detect, lang_list | +| `tools_metrics.go` | metrics | metrics_record, metrics_query | +| `tools_process.go` | process | process_start, process_stop, process_kill, process_list, process_output, process_input | +| `tools_rag.go` | rag | rag_query, rag_ingest, rag_collections | +| `tools_webview.go` | webview | webview_connect, webview_navigate, etc. | +| `tools_ws.go` | ws | ws_start, ws_info | -### PHP package (`src/php/`) +## Subsystems -Three namespace roots mapping to the Laravel request lifecycle: +| Package | Name | Purpose | +|---------|------|---------| +| `pkg/mcp/brain/` | brain | OpenBrain recall, remember, forget | +| `pkg/mcp/ide/` | ide | IDE bridge to Laravel backend | +| `pkg/mcp/agentic/` | agentic | Dispatch, status, plans, PRs, scans | -| Namespace | Path | Role | -|-----------|------|------| -| `Core\Front\Mcp` | `src/Front/Mcp/` | Frontage — middleware group, `McpToolHandler` contract, lifecycle events | -| `Core\Mcp` | `src/Mcp/` | Module — service provider, models, services, tools, admin panel | -| `Core\Website\Mcp` | `src/Website/Mcp/` | Website — playground, API explorer, metrics dashboard | +## Adding a New Tool -Boot chain: `Core\Front\Mcp\Boot` (auto-discovered) fires `McpRoutesRegistering` / `McpToolsRegistering` → `Core\Mcp\Boot` listens and registers routes, tools, admin views, artisan commands. +```go +// 1. Define Input/Output structs +type MyInput struct { + Name string `json:"name"` +} +type MyOutput struct { + Result string `json:"result"` +} -Key services (bound as singletons): `ToolRegistry`, `ToolAnalyticsService`, `McpQuotaService`, `CircuitBreaker`, `AuditLogService`, `QueryExecutionService`. +// 2. Write handler +func (s *Service) myTool(ctx context.Context, req *mcp.CallToolRequest, input MyInput) (*mcp.CallToolResult, MyOutput, error) { + return nil, MyOutput{Result: "done"}, nil +} -`QueryDatabase` tool has 7-layer SQL security (keyword blocking, pattern detection, whitelist, table blocklist, row limits, timeouts, audit logging). +// 3. Register in registerTools() +addToolRecorded(s, server, "group", &mcp.Tool{ + Name: "my_tool", + Description: "Does something useful", +}, s.myTool) +``` -### Brain-seed utility (`cmd/brain-seed/`) +## Adding a New Subsystem -Bulk-imports MEMORY.md, plan docs, and CLAUDE.md files into OpenBrain via the PHP MCP API. Splits by headings, infers memory type, truncates to 3800 chars. +```go +type MySubsystem struct{} -## Conventions +func (m *MySubsystem) Name() string { return "my-sub" } +func (m *MySubsystem) RegisterTools(server *mcp.Server) { + // register tools here +} -- **UK English** in all user-facing strings and docs (colour, organisation, centre, normalise) -- **SPDX headers** in Go files: `// SPDX-License-Identifier: EUPL-1.2` -- **`declare(strict_types=1);`** in every PHP file -- **Full type hints** on all PHP parameters and return types -- **Pest syntax** for PHP tests (not PHPUnit) -- **Flux Pro** components in Livewire views (not vanilla Alpine); **Font Awesome** icons (not Heroicons) -- **Conventional commits**: `type(scope): description` — e.g. `feat(mcp): add new tool` -- Go test names use `_Good` / `_Bad` / `_Ugly` suffixes (happy path / error path / edge cases) +// Register via Options +svc, err := mcp.New(mcp.Options{ + Subsystems: []mcp.Subsystem{&MySubsystem{}}, +}) +``` -## Adding a new Go tool +Subsystems that need to push channel events implement `SubsystemWithNotifier`. -1. Define `Input` and `Output` structs with `json` tags -2. Write handler: `func (s *Service) myTool(ctx, *mcp.CallToolRequest, Input) (*mcp.CallToolResult, Output, error)` -3. Register in `registerTools()`: `addToolRecorded(s, server, "group", &mcp.Tool{...}, s.myTool)` +## Transports -## Adding a new Go subsystem +Selected by `Run()` in priority order: +1. Streamable HTTP (`MCP_HTTP_ADDR` env) — Bearer auth via `MCP_AUTH_TOKEN` +2. TCP (`MCP_ADDR` env) +3. Stdio (default) — used by Claude Code / IDEs -1. Create package under `pkg/mcp/`, implement `Subsystem` (and optionally `SubsystemWithShutdown`) -2. Register: `mcp.New(mcp.WithSubsystem(&mysubsystem.Subsystem{}))` +## Test Naming -## Adding a new PHP tool +`_Good` (happy path), `_Bad` (expected errors), `_Ugly` (panics/edge cases). -1. Implement `Core\Front\Mcp\Contracts\McpToolHandler` (`schema()` + `handle()`) -2. Register via the `McpToolsRegistering` lifecycle event +## Go Workspace -## Key dependencies - -| Go module | Role | -|-----------|------| -| `github.com/modelcontextprotocol/go-sdk` | Official MCP Go SDK | -| `forge.lthn.ai/core/go-io` | Filesystem abstraction + sandboxing | -| `forge.lthn.ai/core/go-ml` | ML inference, scoring, probes | -| `forge.lthn.ai/core/go-rag` | Qdrant vector search | -| `forge.lthn.ai/core/go-process` | Process lifecycle management | -| `forge.lthn.ai/core/api` | REST framework + `ToolBridge` | -| `forge.lthn.ai/core/go-ws` | WebSocket hub | - -PHP: `lthn/php` (Core framework), Laravel 12, Livewire 3, Flux Pro. - -Go workspace: this module is part of `~/Code/go.work`. Requires Go 1.26+, PHP 8.2+. +Part of `~/Code/go.work`. Use `GOWORK=off` to test in isolation. diff --git a/EXCEPTIONS.md b/EXCEPTIONS.md new file mode 100644 index 0000000..71fad0c --- /dev/null +++ b/EXCEPTIONS.md @@ -0,0 +1,17 @@ +# Exceptions + +Items from the Codex review that cannot be fixed, with reasons. + +## 6. Compile-time interface assertions in subsystem packages + +**Files:** `brain/brain.go`, `brain/direct.go`, `agentic/prep.go`, `ide/ide.go` + +**Finding:** Add `var _ Subsystem = (*T)(nil)` compile-time assertions. + +**Reason:** The `Subsystem` interface is defined in the parent `mcp` package. Subsystem packages (`brain`, `agentic`, `ide`) cannot import `mcp` because `mcp` already imports them via `Options.Subsystems` — this would create a circular import. The interface conformance is enforced at runtime when `RegisterTools` is called during `mcp.New()`. + +## 7. Compile-time Notifier assertion on Service + +**Finding:** Add `var _ Notifier = (*Service)(nil)`. + +**Resolution:** Fixed — assertion added to `pkg/mcp/subsystem.go` (where the `Notifier` interface is defined). The TODO originally claimed this was already done in commit `907d62a`, but it was not present in the codebase. diff --git a/cmd/brain-seed/main.go b/cmd/brain-seed/main.go index 3dd5f2a..6616f68 100644 --- a/cmd/brain-seed/main.go +++ b/cmd/brain-seed/main.go @@ -19,7 +19,7 @@ import ( "encoding/json" "flag" "fmt" - "io" + goio "io" "net/http" "os" "path/filepath" @@ -130,7 +130,7 @@ func main() { filename := strings.TrimSuffix(filepath.Base(f), ".md") if len(sections) == 0 { - fmt.Printf(" skip %s/%s (no sections)\n", project, filename) + coreerr.Warn("brain-seed: skip file (no sections)", "project", project, "file", filename) skipped++ continue } @@ -157,7 +157,7 @@ func main() { } if err := callBrainRemember(content, memType, tags, project, confidence); err != nil { - fmt.Printf(" FAIL %s/%s :: %s — %v\n", project, filename, sec.heading, err) + coreerr.Error("brain-seed: import failed", "project", project, "file", filename, "heading", sec.heading, "err", err) errors++ continue } @@ -197,7 +197,7 @@ func main() { } if err := callBrainRemember(content, "plan", tags, project, 0.6); err != nil { - fmt.Printf(" FAIL %s :: %s / %s — %v\n", project, filename, sec.heading, err) + coreerr.Error("brain-seed: plan import failed", "project", project, "file", filename, "heading", sec.heading, "err", err) errors++ continue } @@ -237,7 +237,7 @@ func main() { } if err := callBrainRemember(content, "convention", tags, project, 0.9); err != nil { - fmt.Printf(" FAIL %s :: CLAUDE.md / %s — %v\n", project, sec.heading, err) + coreerr.Error("brain-seed: claude-md import failed", "project", project, "heading", sec.heading, "err", err) errors++ continue } @@ -291,7 +291,7 @@ func callBrainRemember(content, memType string, tags []string, project string, c } defer resp.Body.Close() - respBody, _ := io.ReadAll(resp.Body) + respBody, _ := goio.ReadAll(resp.Body) if resp.StatusCode != 200 { return coreerr.E("callBrainRemember", "HTTP "+string(respBody), nil) diff --git a/cmd/mcpcmd/cmd_mcp.go b/cmd/mcpcmd/cmd_mcp.go index 3c2c1c0..3524dd9 100644 --- a/cmd/mcpcmd/cmd_mcp.go +++ b/cmd/mcpcmd/cmd_mcp.go @@ -61,24 +61,23 @@ func AddMCPCommands(root *cli.Command) { } func runServe() error { - // Build MCP service options - var opts []mcp.Option + opts := mcp.Options{} if workspaceFlag != "" { - opts = append(opts, mcp.WithWorkspaceRoot(workspaceFlag)) + opts.WorkspaceRoot = workspaceFlag } else { // Explicitly unrestricted when no workspace specified - opts = append(opts, mcp.WithWorkspaceRoot("")) + opts.Unrestricted = true } - // Register OpenBrain subsystem (direct HTTP to api.lthn.sh) - opts = append(opts, mcp.WithSubsystem(brain.NewDirect())) - - // Register agentic subsystem (workspace prep, agent orchestration) - opts = append(opts, mcp.WithSubsystem(agentic.NewPrep())) + // Register OpenBrain and agentic subsystems + opts.Subsystems = []mcp.Subsystem{ + brain.NewDirect(), + agentic.NewPrep(), + } // Create the MCP service - svc, err := mcp.New(opts...) + svc, err := mcp.New(opts) if err != nil { return cli.Wrap(err, "create MCP service") } diff --git a/pkg/mcp/agentic/dispatch.go b/pkg/mcp/agentic/dispatch.go index a8ca334..ce01daf 100644 --- a/pkg/mcp/agentic/dispatch.go +++ b/pkg/mcp/agentic/dispatch.go @@ -155,7 +155,19 @@ func (s *PrepSubsystem) dispatch(ctx context.Context, req *mcp.CallToolRequest, }, nil } - // Step 3: Spawn agent as a detached process + // Step 3: Write status BEFORE spawning so concurrent dispatches + // see this workspace as "running" during the concurrency check. + writeStatus(wsDir, &WorkspaceStatus{ + Status: "running", + Agent: input.Agent, + Repo: input.Repo, + Org: input.Org, + Task: input.Task, + StartedAt: time.Now(), + Runs: 1, + }) + + // Step 4: Spawn agent as a detached process // Uses Setpgid so the agent survives parent (MCP server) death. // Output goes directly to log file (not buffered in memory). command, args, err := agentCommand(input.Agent, prompt) @@ -174,7 +186,13 @@ func (s *PrepSubsystem) dispatch(ctx context.Context, req *mcp.CallToolRequest, // - Stdin from /dev/null // - TERM=dumb prevents terminal control sequences // - NO_COLOR=1 disables colour output - devNull, _ := os.Open(os.DevNull) + devNull, err := os.Open(os.DevNull) + if err != nil { + outFile.Close() + return nil, DispatchOutput{}, coreerr.E("dispatch", "failed to open /dev/null", err) + } + defer devNull.Close() + cmd := exec.Command(command, args...) cmd.Dir = srcDir cmd.Stdin = devNull @@ -185,12 +203,19 @@ func (s *PrepSubsystem) dispatch(ctx context.Context, req *mcp.CallToolRequest, if err := cmd.Start(); err != nil { outFile.Close() + // Revert status so the slot is freed + writeStatus(wsDir, &WorkspaceStatus{ + Status: "failed", + Agent: input.Agent, + Repo: input.Repo, + Task: input.Task, + }) return nil, DispatchOutput{}, coreerr.E("dispatch", "failed to spawn "+input.Agent, err) } pid := cmd.Process.Pid - // Write initial status + // Update status with PID now that agent is running writeStatus(wsDir, &WorkspaceStatus{ Status: "running", Agent: input.Agent, @@ -231,3 +256,4 @@ func (s *PrepSubsystem) dispatch(ctx context.Context, req *mcp.CallToolRequest, OutputFile: outputFile, }, nil } + diff --git a/pkg/mcp/agentic/epic.go b/pkg/mcp/agentic/epic.go index 29924ce..2cf83e0 100644 --- a/pkg/mcp/agentic/epic.go +++ b/pkg/mcp/agentic/epic.go @@ -196,10 +196,13 @@ func (s *PrepSubsystem) resolveLabelIDs(ctx context.Context, org, repo string, n req.Header.Set("Authorization", "token "+s.forgeToken) resp, err := s.client.Do(req) - if err != nil || resp.StatusCode != 200 { + if err != nil { return nil } defer resp.Body.Close() + if resp.StatusCode != 200 { + return nil + } var existing []struct { ID int64 `json:"id"` @@ -252,10 +255,13 @@ func (s *PrepSubsystem) createLabel(ctx context.Context, org, repo, name string) req.Header.Set("Authorization", "token "+s.forgeToken) resp, err := s.client.Do(req) - if err != nil || resp.StatusCode != 201 { + if err != nil { return 0 } defer resp.Body.Close() + if resp.StatusCode != 201 { + return 0 + } var result struct { ID int64 `json:"id"` diff --git a/pkg/mcp/agentic/ingest.go b/pkg/mcp/agentic/ingest.go index bb683b6..760ba46 100644 --- a/pkg/mcp/agentic/ingest.go +++ b/pkg/mcp/agentic/ingest.go @@ -23,7 +23,10 @@ func (s *PrepSubsystem) ingestFindings(wsDir string) { } // Read the log file - logFiles, _ := filepath.Glob(filepath.Join(wsDir, "agent-*.log")) + logFiles, err := filepath.Glob(filepath.Join(wsDir, "agent-*.log")) + if err != nil { + return + } if len(logFiles) == 0 { return } diff --git a/pkg/mcp/agentic/plan.go b/pkg/mcp/agentic/plan.go index cf4cf4e..db2cf8d 100644 --- a/pkg/mcp/agentic/plan.go +++ b/pkg/mcp/agentic/plan.go @@ -7,7 +7,6 @@ import ( "crypto/rand" "encoding/hex" "encoding/json" - "os" "path/filepath" "strings" "time" @@ -255,7 +254,7 @@ func (s *PrepSubsystem) planDelete(_ context.Context, _ *mcp.CallToolRequest, in } path := planPath(s.plansDir(), input.ID) - if _, err := os.Stat(path); err != nil { + if !coreio.Local.IsFile(path) { return nil, PlanDeleteOutput{}, coreerr.E("planDelete", "plan not found: "+input.ID, nil) } @@ -275,7 +274,7 @@ func (s *PrepSubsystem) planList(_ context.Context, _ *mcp.CallToolRequest, inpu return nil, PlanListOutput{}, coreerr.E("planList", "failed to access plans directory", err) } - entries, err := os.ReadDir(dir) + entries, err := coreio.Local.List(dir) if err != nil { return nil, PlanListOutput{}, coreerr.E("planList", "failed to read plans directory", err) } @@ -313,8 +312,7 @@ func (s *PrepSubsystem) planList(_ context.Context, _ *mcp.CallToolRequest, inpu // --- Helpers --- func (s *PrepSubsystem) plansDir() string { - home, _ := os.UserHomeDir() - return filepath.Join(home, "Code", "host-uk", "core", ".core", "plans") + return filepath.Join(s.codePath, ".core", "plans") } func planPath(dir, id string) string { @@ -354,7 +352,7 @@ func generatePlanID(title string) string { func readPlan(dir, id string) (*Plan, error) { data, err := coreio.Local.Read(planPath(dir, id)) if err != nil { - return nil, coreerr.E("readPlan", "plan not found: "+id, nil) + return nil, coreerr.E("readPlan", "plan not found: "+id, err) } var plan Plan diff --git a/pkg/mcp/agentic/pr.go b/pkg/mcp/agentic/pr.go index e200de6..1622dc0 100644 --- a/pkg/mcp/agentic/pr.go +++ b/pkg/mcp/agentic/pr.go @@ -8,11 +8,11 @@ import ( "encoding/json" "fmt" "net/http" - "os" "os/exec" "path/filepath" "strings" + coreio "forge.lthn.ai/core/go-io" coreerr "forge.lthn.ai/core/go-log" "github.com/modelcontextprotocol/go-sdk/mcp" ) @@ -54,11 +54,10 @@ func (s *PrepSubsystem) createPR(ctx context.Context, _ *mcp.CallToolRequest, in return nil, CreatePROutput{}, coreerr.E("createPR", "no Forge token configured", nil) } - home, _ := os.UserHomeDir() - wsDir := filepath.Join(home, "Code", "host-uk", "core", ".core", "workspace", input.Workspace) + wsDir := filepath.Join(s.workspaceRoot(), input.Workspace) srcDir := filepath.Join(wsDir, "src") - if _, err := os.Stat(srcDir); err != nil { + if _, err := coreio.Local.List(srcDir); err != nil { return nil, CreatePROutput{}, coreerr.E("createPR", "workspace not found: "+input.Workspace, nil) } @@ -309,10 +308,13 @@ func (s *PrepSubsystem) listRepoPRs(ctx context.Context, org, repo, state string req.Header.Set("Authorization", "token "+s.forgeToken) resp, err := s.client.Do(req) - if err != nil || resp.StatusCode != 200 { + if err != nil { return nil, coreerr.E("listRepoPRs", "failed to list PRs for "+repo, err) } defer resp.Body.Close() + if resp.StatusCode != 200 { + return nil, coreerr.E("listRepoPRs", fmt.Sprintf("HTTP %d for "+repo, resp.StatusCode), nil) + } var prs []struct { Number int `json:"number"` diff --git a/pkg/mcp/agentic/prep.go b/pkg/mcp/agentic/prep.go index a7be87b..cca8857 100644 --- a/pkg/mcp/agentic/prep.go +++ b/pkg/mcp/agentic/prep.go @@ -9,7 +9,7 @@ import ( "encoding/base64" "encoding/json" "fmt" - "io" + goio "io" "net/http" "os" "os/exec" @@ -25,13 +25,13 @@ import ( // PrepSubsystem provides agentic MCP tools. type PrepSubsystem struct { - forgeURL string - forgeToken string - brainURL string - brainKey string - specsPath string - codePath string - client *http.Client + forgeURL string + forgeToken string + brainURL string + brainKey string + specsPath string + codePath string + client *http.Client } // NewPrep creates an agentic subsystem. @@ -51,13 +51,13 @@ func NewPrep() *PrepSubsystem { } return &PrepSubsystem{ - forgeURL: envOr("FORGE_URL", "https://forge.lthn.ai"), - forgeToken: forgeToken, - brainURL: envOr("CORE_BRAIN_URL", "https://api.lthn.sh"), - brainKey: brainKey, - specsPath: envOr("SPECS_PATH", filepath.Join(home, "Code", "host-uk", "specs")), - codePath: envOr("CODE_PATH", filepath.Join(home, "Code")), - client: &http.Client{Timeout: 30 * time.Second}, + forgeURL: envOr("FORGE_URL", "https://forge.lthn.ai"), + forgeToken: forgeToken, + brainURL: envOr("CORE_BRAIN_URL", "https://api.lthn.sh"), + brainKey: brainKey, + specsPath: envOr("SPECS_PATH", filepath.Join(home, "Code", "host-uk", "specs")), + codePath: envOr("CODE_PATH", filepath.Join(home, "Code")), + client: &http.Client{Timeout: 30 * time.Second}, } } @@ -68,6 +68,42 @@ func envOr(key, fallback string) string { return fallback } +func sanitizeRepoPathSegment(value, field string, allowSubdirs bool) (string, error) { + if strings.TrimSpace(value) != value { + return "", coreerr.E("prepWorkspace", field+" contains whitespace", nil) + } + if value == "" { + return "", nil + } + if strings.Contains(value, "\\") { + return "", coreerr.E("prepWorkspace", field+" contains invalid path separator", nil) + } + + parts := strings.Split(value, "/") + if !allowSubdirs && len(parts) != 1 { + return "", coreerr.E("prepWorkspace", field+" may not contain subdirectories", nil) + } + + for _, part := range parts { + if part == "" || part == "." || part == ".." { + return "", coreerr.E("prepWorkspace", field+" contains invalid path segment", nil) + } + for _, r := range part { + switch { + case r >= 'a' && r <= 'z', + r >= 'A' && r <= 'Z', + r >= '0' && r <= '9', + r == '-' || r == '_' || r == '.': + continue + default: + return "", coreerr.E("prepWorkspace", field+" contains invalid characters", nil) + } + } + } + + return value, nil +} + // Name implements mcp.Subsystem. func (s *PrepSubsystem) Name() string { return "agentic" } @@ -96,6 +132,11 @@ func (s *PrepSubsystem) RegisterTools(server *mcp.Server) { // Shutdown implements mcp.SubsystemWithShutdown. func (s *PrepSubsystem) Shutdown(_ context.Context) error { return nil } +// workspaceRoot returns the base directory for agent workspaces. +func (s *PrepSubsystem) workspaceRoot() string { + return filepath.Join(s.codePath, ".core", "workspace") +} + // --- Input/Output types --- // PrepInput is the input for agentic_prep_workspace. @@ -112,20 +153,41 @@ type PrepInput struct { // PrepOutput is the output for agentic_prep_workspace. type PrepOutput struct { - Success bool `json:"success"` - WorkspaceDir string `json:"workspace_dir"` - WikiPages int `json:"wiki_pages"` - SpecFiles int `json:"spec_files"` - Memories int `json:"memories"` - Consumers int `json:"consumers"` - ClaudeMd bool `json:"claude_md"` - GitLog int `json:"git_log_entries"` + Success bool `json:"success"` + WorkspaceDir string `json:"workspace_dir"` + WikiPages int `json:"wiki_pages"` + SpecFiles int `json:"spec_files"` + Memories int `json:"memories"` + Consumers int `json:"consumers"` + ClaudeMd bool `json:"claude_md"` + GitLog int `json:"git_log_entries"` } func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolRequest, input PrepInput) (*mcp.CallToolResult, PrepOutput, error) { if input.Repo == "" { return nil, PrepOutput{}, coreerr.E("prepWorkspace", "repo is required", nil) } + + repo, err := sanitizeRepoPathSegment(input.Repo, "repo", false) + if err != nil { + return nil, PrepOutput{}, err + } + input.Repo = repo + + planTemplate, err := sanitizeRepoPathSegment(input.PlanTemplate, "plan_template", false) + if err != nil { + return nil, PrepOutput{}, err + } + input.PlanTemplate = planTemplate + + persona := input.Persona + if persona != "" { + persona, err = sanitizeRepoPathSegment(persona, "persona", true) + if err != nil { + return nil, PrepOutput{}, err + } + } + if input.Org == "" { input.Org = "core" } @@ -134,8 +196,7 @@ func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolReques } // Workspace root: .core/workspace/{repo}-{timestamp}/ - home, _ := os.UserHomeDir() - wsRoot := filepath.Join(home, "Code", "host-uk", "core", ".core", "workspace") + wsRoot := s.workspaceRoot() wsName := fmt.Sprintf("%s-%d", input.Repo, time.Now().Unix()) wsDir := filepath.Join(wsRoot, wsName) @@ -150,7 +211,9 @@ func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolReques // 1. Clone repo into src/ and create feature branch srcDir := filepath.Join(wsDir, "src") cloneCmd := exec.CommandContext(ctx, "git", "clone", repoPath, srcDir) - cloneCmd.Run() + if err := cloneCmd.Run(); err != nil { + return nil, PrepOutput{}, coreerr.E("prepWorkspace", "failed to clone repository", err) + } // Create feature branch taskSlug := strings.Map(func(r rune) rune { @@ -166,11 +229,14 @@ func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolReques taskSlug = taskSlug[:40] } taskSlug = strings.Trim(taskSlug, "-") - branchName := fmt.Sprintf("agent/%s", taskSlug) - - branchCmd := exec.CommandContext(ctx, "git", "checkout", "-b", branchName) - branchCmd.Dir = srcDir - branchCmd.Run() + if taskSlug != "" { + branchName := fmt.Sprintf("agent/%s", taskSlug) + branchCmd := exec.CommandContext(ctx, "git", "checkout", "-b", branchName) + branchCmd.Dir = srcDir + if err := branchCmd.Run(); err != nil { + return nil, PrepOutput{}, coreerr.E("prepWorkspace", "failed to create branch", err) + } + } // Create context dirs inside src/ coreio.Local.EnsureDir(filepath.Join(srcDir, "kb")) @@ -192,8 +258,8 @@ func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolReques } // Copy persona if specified - if input.Persona != "" { - personaPath := filepath.Join(s.codePath, "core", "agent", "prompts", "personas", input.Persona+".md") + if persona != "" { + personaPath := filepath.Join(s.codePath, "core", "agent", "prompts", "personas", persona+".md") if data, err := coreio.Local.Read(personaPath); err == nil { coreio.Local.Write(filepath.Join(wsDir, "src", "PERSONA.md"), data) } @@ -334,9 +400,9 @@ func (s *PrepSubsystem) writePlanFromTemplate(templateSlug string, variables map Description string `yaml:"description"` Guidelines []string `yaml:"guidelines"` Phases []struct { - Name string `yaml:"name"` - Description string `yaml:"description"` - Tasks []any `yaml:"tasks"` + Name string `yaml:"name"` + Description string `yaml:"description"` + Tasks []any `yaml:"tasks"` } `yaml:"phases"` } @@ -395,10 +461,13 @@ func (s *PrepSubsystem) pullWiki(ctx context.Context, org, repo, wsDir string) i req.Header.Set("Authorization", "token "+s.forgeToken) resp, err := s.client.Do(req) - if err != nil || resp.StatusCode != 200 { + if err != nil { return 0 } defer resp.Body.Close() + if resp.StatusCode != 200 { + return 0 + } var pages []struct { Title string `json:"title"` @@ -418,7 +487,11 @@ func (s *PrepSubsystem) pullWiki(ctx context.Context, org, repo, wsDir string) i pageReq.Header.Set("Authorization", "token "+s.forgeToken) pageResp, err := s.client.Do(pageReq) - if err != nil || pageResp.StatusCode != 200 { + if err != nil { + continue + } + if pageResp.StatusCode != 200 { + pageResp.Body.Close() continue } @@ -480,12 +553,15 @@ func (s *PrepSubsystem) generateContext(ctx context.Context, repo, wsDir string) req.Header.Set("Authorization", "Bearer "+s.brainKey) resp, err := s.client.Do(req) - if err != nil || resp.StatusCode != 200 { + if err != nil { return 0 } defer resp.Body.Close() + if resp.StatusCode != 200 { + return 0 + } - respData, _ := io.ReadAll(resp.Body) + respData, _ := goio.ReadAll(resp.Body) var result struct { Memories []map[string]any `json:"memories"` } @@ -573,10 +649,13 @@ func (s *PrepSubsystem) generateTodo(ctx context.Context, org, repo string, issu req.Header.Set("Authorization", "token "+s.forgeToken) resp, err := s.client.Do(req) - if err != nil || resp.StatusCode != 200 { + if err != nil { return } defer resp.Body.Close() + if resp.StatusCode != 200 { + return + } var issueData struct { Title string `json:"title"` diff --git a/pkg/mcp/agentic/prep_test.go b/pkg/mcp/agentic/prep_test.go new file mode 100644 index 0000000..51f3a60 --- /dev/null +++ b/pkg/mcp/agentic/prep_test.go @@ -0,0 +1,96 @@ +// SPDX-License-Identifier: EUPL-1.2 + +package agentic + +import ( + "context" + "strings" + "testing" +) + +func TestSanitizeRepoPathSegment_Good(t *testing.T) { + t.Run("repo", func(t *testing.T) { + value, err := sanitizeRepoPathSegment("go-io", "repo", false) + if err != nil { + t.Fatalf("expected valid repo name, got error: %v", err) + } + if value != "go-io" { + t.Fatalf("expected normalized value, got: %q", value) + } + }) + + t.Run("persona", func(t *testing.T) { + value, err := sanitizeRepoPathSegment("engineering/backend-architect", "persona", true) + if err != nil { + t.Fatalf("expected valid persona path, got error: %v", err) + } + if value != "engineering/backend-architect" { + t.Fatalf("expected persona path, got: %q", value) + } + }) +} + +func TestSanitizeRepoPathSegment_Bad(t *testing.T) { + cases := []struct { + name string + value string + allowPath bool + }{ + {"repo segment traversal", "../repo", false}, + {"repo nested path", "team/repo", false}, + {"plan template traversal", "../secret", false}, + {"persona traversal", "engineering/../../admin", true}, + {"backslash", "org\\repo", false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := sanitizeRepoPathSegment(tc.value, tc.name, tc.allowPath) + if err == nil { + t.Fatal("expected error") + } + }) + } +} + +func TestPrepWorkspace_Bad_BadRepoTraversal(t *testing.T) { + s := &PrepSubsystem{codePath: t.TempDir()} + + _, _, err := s.prepWorkspace(context.Background(), nil, PrepInput{Repo: "../repo"}) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(strings.ToLower(err.Error()), "repo") { + t.Fatalf("expected repo error, got %q", err) + } +} + +func TestPrepWorkspace_Bad_BadPersonaTraversal(t *testing.T) { + s := &PrepSubsystem{codePath: t.TempDir()} + + _, _, err := s.prepWorkspace(context.Background(), nil, PrepInput{ + Repo: "repo", + Persona: "engineering/../../admin", + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(strings.ToLower(err.Error()), "persona") { + t.Fatalf("expected persona error, got %q", err) + } +} + +func TestPrepWorkspace_Bad_BadPlanTemplateTraversal(t *testing.T) { + s := &PrepSubsystem{codePath: t.TempDir()} + + _, _, err := s.prepWorkspace(context.Background(), nil, PrepInput{ + Repo: "repo", + PlanTemplate: "../secret", + }) + if err == nil { + t.Fatal("expected error") + } + if !strings.Contains(strings.ToLower(err.Error()), "plan_template") { + t.Fatalf("expected plan template error, got %q", err) + } +} diff --git a/pkg/mcp/agentic/queue.go b/pkg/mcp/agentic/queue.go index f42add2..b8a8972 100644 --- a/pkg/mcp/agentic/queue.go +++ b/pkg/mcp/agentic/queue.go @@ -43,9 +43,7 @@ type AgentsConfig struct { // loadAgentsConfig reads config/agents.yaml from the code path. func (s *PrepSubsystem) loadAgentsConfig() *AgentsConfig { paths := []string{ - filepath.Join(s.codePath, "core", "agent", "config", "agents.yaml"), - filepath.Join(s.codePath, "core", "agent", ".core", "agents.yaml"), - filepath.Join(s.codePath, "host-uk", "core", ".core", "agents.yaml"), + filepath.Join(s.codePath, ".core", "agents.yaml"), } for _, path := range paths { @@ -103,32 +101,55 @@ func (s *PrepSubsystem) delayForAgent(agent string) time.Duration { return time.Duration(rate.SustainedDelay) * time.Second } -// countRunningByAgent counts running workspaces for a specific agent type. -func (s *PrepSubsystem) countRunningByAgent(agent string) int { - home, _ := os.UserHomeDir() - wsRoot := filepath.Join(home, "Code", "host-uk", "core", ".core", "workspace") - - entries, err := os.ReadDir(wsRoot) +// listWorkspaceDirs returns all workspace directories, including those +// nested one level deep (e.g. workspace/core/go-io-123/). +func (s *PrepSubsystem) listWorkspaceDirs() []string { + wsRoot := s.workspaceRoot() + entries, err := coreio.Local.List(wsRoot) if err != nil { - return 0 + return nil } - count := 0 + var dirs []string for _, entry := range entries { if !entry.IsDir() { continue } + path := filepath.Join(wsRoot, entry.Name()) + // Check if this dir has a status.json (it's a workspace) + if coreio.Local.IsFile(filepath.Join(path, "status.json")) { + dirs = append(dirs, path) + continue + } + // Otherwise check one level deeper (org subdirectory) + subEntries, err := coreio.Local.List(path) + if err != nil { + continue + } + for _, sub := range subEntries { + if sub.IsDir() { + subPath := filepath.Join(path, sub.Name()) + if coreio.Local.IsFile(filepath.Join(subPath, "status.json")) { + dirs = append(dirs, subPath) + } + } + } + } + return dirs +} - st, err := readStatus(filepath.Join(wsRoot, entry.Name())) +// countRunningByAgent counts running workspaces for a specific agent type. +func (s *PrepSubsystem) countRunningByAgent(agent string) int { + count := 0 + for _, wsDir := range s.listWorkspaceDirs() { + st, err := readStatus(wsDir) if err != nil || st.Status != "running" { continue } - // Match on base agent type (gemini:flash matches gemini) stBase := strings.SplitN(st.Agent, ":", 2)[0] if stBase != agent { continue } - if st.PID > 0 { proc, err := os.FindProcess(st.PID) if err == nil && proc.Signal(syscall.Signal(0)) == nil { @@ -136,7 +157,6 @@ func (s *PrepSubsystem) countRunningByAgent(agent string) int { } } } - return count } @@ -164,20 +184,7 @@ func (s *PrepSubsystem) canDispatch() bool { // drainQueue finds the oldest queued workspace and spawns it if a slot is available. // Applies rate-based delay between spawns. func (s *PrepSubsystem) drainQueue() { - home, _ := os.UserHomeDir() - wsRoot := filepath.Join(home, "Code", "host-uk", "core", ".core", "workspace") - - entries, err := os.ReadDir(wsRoot) - if err != nil { - return - } - - for _, entry := range entries { - if !entry.IsDir() { - continue - } - - wsDir := filepath.Join(wsRoot, entry.Name()) + for _, wsDir := range s.listWorkspaceDirs() { st, err := readStatus(wsDir) if err != nil || st.Status != "queued" { continue @@ -212,7 +219,12 @@ func (s *PrepSubsystem) drainQueue() { continue } - devNull, _ := os.Open(os.DevNull) + devNull, err := os.Open(os.DevNull) + if err != nil { + outFile.Close() + continue + } + cmd := exec.Command(command, args...) cmd.Dir = srcDir cmd.Stdin = devNull @@ -223,8 +235,10 @@ func (s *PrepSubsystem) drainQueue() { if err := cmd.Start(); err != nil { outFile.Close() + devNull.Close() continue } + devNull.Close() st.Status = "running" st.PID = cmd.Process.Pid diff --git a/pkg/mcp/agentic/resume.go b/pkg/mcp/agentic/resume.go index dba9de8..0340adb 100644 --- a/pkg/mcp/agentic/resume.go +++ b/pkg/mcp/agentic/resume.go @@ -45,12 +45,11 @@ func (s *PrepSubsystem) resume(ctx context.Context, _ *mcp.CallToolRequest, inpu return nil, ResumeOutput{}, coreerr.E("resume", "workspace is required", nil) } - home, _ := os.UserHomeDir() - wsDir := filepath.Join(home, "Code", "host-uk", "core", ".core", "workspace", input.Workspace) + wsDir := filepath.Join(s.workspaceRoot(), input.Workspace) srcDir := filepath.Join(wsDir, "src") // Verify workspace exists - if _, err := os.Stat(srcDir); err != nil { + if _, err := coreio.Local.List(srcDir); err != nil { return nil, ResumeOutput{}, coreerr.E("resume", "workspace not found: "+input.Workspace, nil) } @@ -103,8 +102,17 @@ func (s *PrepSubsystem) resume(ctx context.Context, _ *mcp.CallToolRequest, inpu return nil, ResumeOutput{}, err } - devNull, _ := os.Open(os.DevNull) - outFile, _ := os.Create(outputFile) + devNull, err := os.Open(os.DevNull) + if err != nil { + return nil, ResumeOutput{}, coreerr.E("resume", "failed to open /dev/null", err) + } + defer devNull.Close() + + outFile, err := os.Create(outputFile) + if err != nil { + return nil, ResumeOutput{}, coreerr.E("resume", "failed to create log file", err) + } + cmd := exec.Command(command, args...) cmd.Dir = srcDir cmd.Stdin = devNull diff --git a/pkg/mcp/agentic/scan.go b/pkg/mcp/agentic/scan.go index ca78717..4eb741e 100644 --- a/pkg/mcp/agentic/scan.go +++ b/pkg/mcp/agentic/scan.go @@ -105,10 +105,13 @@ func (s *PrepSubsystem) listOrgRepos(ctx context.Context, org string) ([]string, req.Header.Set("Authorization", "token "+s.forgeToken) resp, err := s.client.Do(req) - if err != nil || resp.StatusCode != 200 { + if err != nil { return nil, coreerr.E("listOrgRepos", "failed to list repos", err) } defer resp.Body.Close() + if resp.StatusCode != 200 { + return nil, coreerr.E("listOrgRepos", fmt.Sprintf("HTTP %d listing repos", resp.StatusCode), nil) + } var repos []struct { Name string `json:"name"` @@ -129,10 +132,13 @@ func (s *PrepSubsystem) listRepoIssues(ctx context.Context, org, repo, label str req.Header.Set("Authorization", "token "+s.forgeToken) resp, err := s.client.Do(req) - if err != nil || resp.StatusCode != 200 { + if err != nil { return nil, coreerr.E("listRepoIssues", "failed to list issues for "+repo, err) } defer resp.Body.Close() + if resp.StatusCode != 200 { + return nil, coreerr.E("listRepoIssues", fmt.Sprintf("HTTP %d for "+repo, resp.StatusCode), nil) + } var issues []struct { Number int `json:"number"` diff --git a/pkg/mcp/agentic/status.go b/pkg/mcp/agentic/status.go index db30b33..980c2f0 100644 --- a/pkg/mcp/agentic/status.go +++ b/pkg/mcp/agentic/status.go @@ -29,19 +29,19 @@ import ( // WorkspaceStatus represents the current state of an agent workspace. type WorkspaceStatus struct { - Status string `json:"status"` // running, completed, blocked, failed - Agent string `json:"agent"` // gemini, claude, codex - Repo string `json:"repo"` // target repo - Org string `json:"org,omitempty"` // forge org (e.g. "core") - Task string `json:"task"` // task description - Branch string `json:"branch,omitempty"` // git branch name - Issue int `json:"issue,omitempty"` // forge issue number - PID int `json:"pid,omitempty"` // process ID (if running) - StartedAt time.Time `json:"started_at"` // when dispatch started - UpdatedAt time.Time `json:"updated_at"` // last status change - Question string `json:"question,omitempty"` // from BLOCKED.md - Runs int `json:"runs"` // how many times dispatched/resumed - PRURL string `json:"pr_url,omitempty"` // pull request URL (after PR created) + Status string `json:"status"` // running, completed, blocked, failed + Agent string `json:"agent"` // gemini, claude, codex + Repo string `json:"repo"` // target repo + Org string `json:"org,omitempty"` // forge org (e.g. "core") + Task string `json:"task"` // task description + Branch string `json:"branch,omitempty"` // git branch name + Issue int `json:"issue,omitempty"` // forge issue number + PID int `json:"pid,omitempty"` // process ID (if running) + StartedAt time.Time `json:"started_at"` // when dispatch started + UpdatedAt time.Time `json:"updated_at"` // last status change + Question string `json:"question,omitempty"` // from BLOCKED.md + Runs int `json:"runs"` // how many times dispatched/resumed + PRURL string `json:"pr_url,omitempty"` // pull request URL (after PR created) } func writeStatus(wsDir string, status *WorkspaceStatus) error { @@ -95,29 +95,21 @@ func (s *PrepSubsystem) registerStatusTool(server *mcp.Server) { } func (s *PrepSubsystem) status(ctx context.Context, _ *mcp.CallToolRequest, input StatusInput) (*mcp.CallToolResult, StatusOutput, error) { - home, _ := os.UserHomeDir() - wsRoot := filepath.Join(home, "Code", "host-uk", "core", ".core", "workspace") - - entries, err := os.ReadDir(wsRoot) - if err != nil { - return nil, StatusOutput{}, coreerr.E("status", "no workspaces found", err) + wsDirs := s.listWorkspaceDirs() + if len(wsDirs) == 0 { + return nil, StatusOutput{}, coreerr.E("status", "no workspaces found", nil) } var workspaces []WorkspaceInfo - for _, entry := range entries { - if !entry.IsDir() { - continue - } - - name := entry.Name() + for _, wsDir := range wsDirs { + name := filepath.Base(wsDir) // Filter by specific workspace if requested if input.Workspace != "" && name != input.Workspace { continue } - wsDir := filepath.Join(wsRoot, name) info := WorkspaceInfo{Name: name} // Try reading status.json @@ -130,8 +122,7 @@ func (s *PrepSubsystem) status(ctx context.Context, _ *mcp.CallToolRequest, inpu } else { info.Status = "unknown" } - fi, _ := entry.Info() - if fi != nil { + if fi, err := os.Stat(wsDir); err == nil { info.Age = time.Since(fi.ModTime()).Truncate(time.Minute).String() } workspaces = append(workspaces, info) diff --git a/pkg/mcp/brain/direct.go b/pkg/mcp/brain/direct.go index de4cb2d..3a7115f 100644 --- a/pkg/mcp/brain/direct.go +++ b/pkg/mcp/brain/direct.go @@ -7,7 +7,7 @@ import ( "context" "encoding/json" "fmt" - "io" + goio "io" "net/http" "os" "strings" @@ -18,13 +18,27 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" ) +// channelSender is the callback for pushing channel events. +type channelSender func(ctx context.Context, channel string, data any) + // DirectSubsystem implements mcp.Subsystem for OpenBrain via direct HTTP calls. // Unlike Subsystem (which uses the IDE WebSocket bridge), this calls the // Laravel API directly — suitable for standalone core-mcp usage. type DirectSubsystem struct { - apiURL string - apiKey string - client *http.Client + apiURL string + apiKey string + client *http.Client + onChannel channelSender +} + +// OnChannel sets a callback for channel event broadcasting. +// Called by the MCP service after creation to wire up notifications. +// +// brain.OnChannel(func(ctx context.Context, ch string, data any) { +// mcpService.ChannelSend(ctx, ch, data) +// }) +func (s *DirectSubsystem) OnChannel(fn func(ctx context.Context, channel string, data any)) { + s.onChannel = fn } // NewDirect creates a brain subsystem that calls the OpenBrain API directly. @@ -79,7 +93,7 @@ func (s *DirectSubsystem) apiCall(ctx context.Context, method, path string, body return nil, coreerr.E("brain.apiCall", "no API key (set CORE_BRAIN_KEY or create ~/.claude/brain.key)", nil) } - var reqBody io.Reader + var reqBody goio.Reader if body != nil { data, err := json.Marshal(body) if err != nil { @@ -102,7 +116,7 @@ func (s *DirectSubsystem) apiCall(ctx context.Context, method, path string, body } defer resp.Body.Close() - respData, err := io.ReadAll(resp.Body) + respData, err := goio.ReadAll(resp.Body) if err != nil { return nil, coreerr.E("brain.apiCall", "read response", err) } @@ -132,6 +146,13 @@ func (s *DirectSubsystem) remember(ctx context.Context, _ *mcp.CallToolRequest, } id, _ := result["id"].(string) + if s.onChannel != nil { + s.onChannel(ctx, "brain.remember.complete", map[string]any{ + "id": id, + "type": input.Type, + "project": input.Project, + }) + } return nil, RememberOutput{ Success: true, MemoryID: id, @@ -185,6 +206,12 @@ func (s *DirectSubsystem) recall(ctx context.Context, _ *mcp.CallToolRequest, in } } + if s.onChannel != nil { + s.onChannel(ctx, "brain.recall.complete", map[string]any{ + "query": input.Query, + "count": len(memories), + }) + } return nil, RecallOutput{ Success: true, Count: len(memories), diff --git a/pkg/mcp/brain/direct_test.go b/pkg/mcp/brain/direct_test.go new file mode 100644 index 0000000..db3c5bd --- /dev/null +++ b/pkg/mcp/brain/direct_test.go @@ -0,0 +1,305 @@ +// SPDX-License-Identifier: EUPL-1.2 + +package brain + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +// newTestDirect creates a DirectSubsystem pointing at a test server. +func newTestDirect(url string) *DirectSubsystem { + return &DirectSubsystem{ + apiURL: url, + apiKey: "test-key", + client: http.DefaultClient, + } +} + +// --- DirectSubsystem interface tests --- + +func TestDirectSubsystem_Good_Name(t *testing.T) { + s := &DirectSubsystem{} + if s.Name() != "brain" { + t.Errorf("expected Name() = 'brain', got %q", s.Name()) + } +} + +func TestDirectSubsystem_Good_Shutdown(t *testing.T) { + s := &DirectSubsystem{} + if err := s.Shutdown(context.Background()); err != nil { + t.Errorf("Shutdown failed: %v", err) + } +} + +// --- apiCall tests --- + +func TestApiCall_Good_PostWithBody(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" { + t.Errorf("expected POST, got %s", r.Method) + } + if r.Header.Get("Authorization") != "Bearer test-key" { + t.Errorf("missing or wrong Authorization header") + } + if r.Header.Get("Content-Type") != "application/json" { + t.Errorf("missing Content-Type header") + } + w.WriteHeader(200) + json.NewEncoder(w).Encode(map[string]any{"id": "mem-123", "success": true}) + })) + defer srv.Close() + + s := newTestDirect(srv.URL) + result, err := s.apiCall(context.Background(), "POST", "/v1/brain/remember", map[string]string{"content": "test"}) + if err != nil { + t.Fatalf("apiCall failed: %v", err) + } + if result["id"] != "mem-123" { + t.Errorf("expected id=mem-123, got %v", result["id"]) + } +} + +func TestApiCall_Good_GetNilBody(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + t.Errorf("expected GET, got %s", r.Method) + } + w.WriteHeader(200) + json.NewEncoder(w).Encode(map[string]any{"status": "ok"}) + })) + defer srv.Close() + + s := newTestDirect(srv.URL) + result, err := s.apiCall(context.Background(), "GET", "/status", nil) + if err != nil { + t.Fatalf("apiCall failed: %v", err) + } + if result["status"] != "ok" { + t.Errorf("expected status=ok, got %v", result["status"]) + } +} + +func TestApiCall_Bad_NoApiKey(t *testing.T) { + s := &DirectSubsystem{apiKey: "", client: http.DefaultClient} + _, err := s.apiCall(context.Background(), "GET", "/test", nil) + if err == nil { + t.Error("expected error when apiKey is empty") + } +} + +func TestApiCall_Bad_HttpError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(500) + w.Write([]byte(`{"error":"internal server error"}`)) + })) + defer srv.Close() + + s := newTestDirect(srv.URL) + _, err := s.apiCall(context.Background(), "POST", "/fail", map[string]string{}) + if err == nil { + t.Error("expected error on HTTP 500") + } +} + +func TestApiCall_Bad_InvalidJson(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + w.Write([]byte("not json")) + })) + defer srv.Close() + + s := newTestDirect(srv.URL) + _, err := s.apiCall(context.Background(), "GET", "/bad-json", nil) + if err == nil { + t.Error("expected error on invalid JSON response") + } +} + +func TestApiCall_Bad_Unreachable(t *testing.T) { + s := &DirectSubsystem{ + apiURL: "http://127.0.0.1:1", // nothing listening + apiKey: "key", + client: http.DefaultClient, + } + _, err := s.apiCall(context.Background(), "GET", "/test", nil) + if err == nil { + t.Error("expected error for unreachable server") + } +} + +// --- remember tool tests --- + +func TestDirectRemember_Good(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + json.NewDecoder(r.Body).Decode(&body) + if body["content"] != "test memory" { + t.Errorf("unexpected content: %v", body["content"]) + } + if body["agent_id"] != "cladius" { + t.Errorf("expected agent_id=cladius, got %v", body["agent_id"]) + } + w.WriteHeader(200) + json.NewEncoder(w).Encode(map[string]any{"id": "mem-456"}) + })) + defer srv.Close() + + s := newTestDirect(srv.URL) + _, out, err := s.remember(context.Background(), nil, RememberInput{ + Content: "test memory", + Type: "observation", + Project: "test-project", + }) + if err != nil { + t.Fatalf("remember failed: %v", err) + } + if !out.Success { + t.Error("expected success=true") + } + if out.MemoryID != "mem-456" { + t.Errorf("expected memoryId=mem-456, got %q", out.MemoryID) + } +} + +func TestDirectRemember_Bad_ApiError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(422) + w.Write([]byte(`{"error":"validation failed"}`)) + })) + defer srv.Close() + + s := newTestDirect(srv.URL) + _, _, err := s.remember(context.Background(), nil, RememberInput{Content: "x", Type: "bug"}) + if err == nil { + t.Error("expected error on API failure") + } +} + +// --- recall tool tests --- + +func TestDirectRecall_Good(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + json.NewDecoder(r.Body).Decode(&body) + if body["query"] != "scoring algorithm" { + t.Errorf("unexpected query: %v", body["query"]) + } + w.WriteHeader(200) + json.NewEncoder(w).Encode(map[string]any{ + "memories": []any{ + map[string]any{ + "id": "mem-1", + "content": "scoring uses weighted average", + "type": "architecture", + "project": "eaas", + "agent_id": "virgil", + "score": 0.92, + "created_at": "2026-03-01T00:00:00Z", + }, + }, + }) + })) + defer srv.Close() + + s := newTestDirect(srv.URL) + _, out, err := s.recall(context.Background(), nil, RecallInput{ + Query: "scoring algorithm", + TopK: 5, + Filter: RecallFilter{Project: "eaas"}, + }) + if err != nil { + t.Fatalf("recall failed: %v", err) + } + if !out.Success || out.Count != 1 { + t.Errorf("expected 1 memory, got %d", out.Count) + } + if out.Memories[0].ID != "mem-1" { + t.Errorf("expected id=mem-1, got %q", out.Memories[0].ID) + } + if out.Memories[0].Confidence != 0.92 { + t.Errorf("expected score=0.92, got %f", out.Memories[0].Confidence) + } +} + +func TestDirectRecall_Good_DefaultTopK(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + json.NewDecoder(r.Body).Decode(&body) + // TopK=0 should default to 10 + if topK, ok := body["top_k"].(float64); !ok || topK != 10 { + t.Errorf("expected top_k=10, got %v", body["top_k"]) + } + w.WriteHeader(200) + json.NewEncoder(w).Encode(map[string]any{"memories": []any{}}) + })) + defer srv.Close() + + s := newTestDirect(srv.URL) + _, out, err := s.recall(context.Background(), nil, RecallInput{Query: "test"}) + if err != nil { + t.Fatalf("recall failed: %v", err) + } + if !out.Success || out.Count != 0 { + t.Errorf("expected empty result, got %d", out.Count) + } +} + +func TestDirectRecall_Bad_ApiError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(500) + w.Write([]byte(`{"error":"internal"}`)) + })) + defer srv.Close() + + s := newTestDirect(srv.URL) + _, _, err := s.recall(context.Background(), nil, RecallInput{Query: "test"}) + if err == nil { + t.Error("expected error on API failure") + } +} + +// --- forget tool tests --- + +func TestDirectForget_Good(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "DELETE" { + t.Errorf("expected DELETE, got %s", r.Method) + } + if r.URL.Path != "/v1/brain/forget/mem-789" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.WriteHeader(200) + json.NewEncoder(w).Encode(map[string]any{"success": true}) + })) + defer srv.Close() + + s := newTestDirect(srv.URL) + _, out, err := s.forget(context.Background(), nil, ForgetInput{ + ID: "mem-789", + Reason: "outdated", + }) + if err != nil { + t.Fatalf("forget failed: %v", err) + } + if !out.Success || out.Forgotten != "mem-789" { + t.Errorf("unexpected output: %+v", out) + } +} + +func TestDirectForget_Bad_ApiError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(404) + w.Write([]byte(`{"error":"not found"}`)) + })) + defer srv.Close() + + s := newTestDirect(srv.URL) + _, _, err := s.forget(context.Background(), nil, ForgetInput{ID: "nonexistent"}) + if err == nil { + t.Error("expected error on 404") + } +} diff --git a/pkg/mcp/brain/provider.go b/pkg/mcp/brain/provider.go index 3dec757..1a02cb1 100644 --- a/pkg/mcp/brain/provider.go +++ b/pkg/mcp/brain/provider.go @@ -1,4 +1,4 @@ -// SPDX-Licence-Identifier: EUPL-1.2 +// SPDX-License-Identifier: EUPL-1.2 package brain diff --git a/pkg/mcp/brain/tools.go b/pkg/mcp/brain/tools.go index 47d1e02..f0d68b9 100644 --- a/pkg/mcp/brain/tools.go +++ b/pkg/mcp/brain/tools.go @@ -200,13 +200,17 @@ func (s *Subsystem) brainList(_ context.Context, _ *mcp.CallToolRequest, input L return nil, ListOutput{}, errBridgeNotAvailable } + limit := input.Limit + if limit == 0 { + limit = 50 // sensible default — backend clamps 0 to 1 + } err := s.bridge.Send(ide.BridgeMessage{ Type: "brain_list", Data: map[string]any{ "project": input.Project, "type": input.Type, "agent_id": input.AgentID, - "limit": input.Limit, + "limit": limit, }, }) if err != nil { diff --git a/pkg/mcp/bridge.go b/pkg/mcp/bridge.go index de02734..4400b3e 100644 --- a/pkg/mcp/bridge.go +++ b/pkg/mcp/bridge.go @@ -5,7 +5,7 @@ package mcp import ( "encoding/json" "errors" - "io" + goio "io" "net/http" "github.com/gin-gonic/gin" @@ -20,6 +20,10 @@ const maxBodySize = 10 << 20 // 10 MB // Each tool becomes a POST endpoint that reads a JSON body, dispatches // to the tool's RESTHandler (which knows the concrete input type), and // wraps the result in the standard api.Response envelope. +// +// bridge := api.NewToolBridge() +// mcp.BridgeToAPI(svc, bridge) +// bridge.Mount(router, "/v1/tools") func BridgeToAPI(svc *Service, bridge *api.ToolBridge) { for rec := range svc.ToolsSeq() { desc := api.ToolDescriptor{ @@ -37,7 +41,7 @@ func BridgeToAPI(svc *Service, bridge *api.ToolBridge) { var body []byte if c.Request.Body != nil { var err error - body, err = io.ReadAll(io.LimitReader(c.Request.Body, maxBodySize)) + body, err = goio.ReadAll(goio.LimitReader(c.Request.Body, maxBodySize)) if err != nil { c.JSON(http.StatusBadRequest, api.Fail("invalid_request", "Failed to read request body")) return diff --git a/pkg/mcp/bridge_test.go b/pkg/mcp/bridge_test.go index a209e25..1db2a07 100644 --- a/pkg/mcp/bridge_test.go +++ b/pkg/mcp/bridge_test.go @@ -21,7 +21,7 @@ func init() { } func TestBridgeToAPI_Good_AllTools(t *testing.T) { - svc, err := New(WithWorkspaceRoot(t.TempDir())) + svc, err := New(Options{WorkspaceRoot: t.TempDir()}) if err != nil { t.Fatal(err) } @@ -52,7 +52,7 @@ func TestBridgeToAPI_Good_AllTools(t *testing.T) { } func TestBridgeToAPI_Good_DescribableGroup(t *testing.T) { - svc, err := New(WithWorkspaceRoot(t.TempDir())) + svc, err := New(Options{WorkspaceRoot: t.TempDir()}) if err != nil { t.Fatal(err) } @@ -90,7 +90,7 @@ func TestBridgeToAPI_Good_FileRead(t *testing.T) { t.Fatal(err) } - svc, err := New(WithWorkspaceRoot(tmpDir)) + svc, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatal(err) } @@ -130,7 +130,7 @@ func TestBridgeToAPI_Good_FileRead(t *testing.T) { } func TestBridgeToAPI_Bad_InvalidJSON(t *testing.T) { - svc, err := New(WithWorkspaceRoot(t.TempDir())) + svc, err := New(Options{WorkspaceRoot: t.TempDir()}) if err != nil { t.Fatal(err) } @@ -170,7 +170,7 @@ func TestBridgeToAPI_Bad_InvalidJSON(t *testing.T) { } func TestBridgeToAPI_Good_EndToEnd(t *testing.T) { - svc, err := New(WithWorkspaceRoot(t.TempDir())) + svc, err := New(Options{WorkspaceRoot: t.TempDir()}) if err != nil { t.Fatal(err) } diff --git a/pkg/mcp/integration_test.go b/pkg/mcp/integration_test.go index de35e66..4fcdee6 100644 --- a/pkg/mcp/integration_test.go +++ b/pkg/mcp/integration_test.go @@ -11,7 +11,7 @@ import ( func TestIntegration_FileTools(t *testing.T) { tmpDir := t.TempDir() - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) assert.NoError(t, err) ctx := context.Background() @@ -85,7 +85,7 @@ func TestIntegration_FileTools(t *testing.T) { func TestIntegration_ErrorPaths(t *testing.T) { tmpDir := t.TempDir() - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) assert.NoError(t, err) ctx := context.Background() diff --git a/pkg/mcp/iter_test.go b/pkg/mcp/iter_test.go index 5c9b274..d4d3d12 100644 --- a/pkg/mcp/iter_test.go +++ b/pkg/mcp/iter_test.go @@ -8,7 +8,7 @@ import ( ) func TestService_Iterators(t *testing.T) { - svc, err := New(WithWorkspaceRoot(t.TempDir())) + svc, err := New(Options{WorkspaceRoot: t.TempDir()}) if err != nil { t.Fatal(err) } diff --git a/pkg/mcp/mcp.go b/pkg/mcp/mcp.go index 38a15b5..114cf40 100644 --- a/pkg/mcp/mcp.go +++ b/pkg/mcp/mcp.go @@ -12,6 +12,7 @@ import ( "path/filepath" "slices" "strings" + "sync" "forge.lthn.ai/core/go-io" "forge.lthn.ai/core/go-log" @@ -22,6 +23,9 @@ import ( // Service provides a lightweight MCP server with file operations only. // For full GUI features, use the core-gui package. +// +// svc, err := mcp.New(mcp.Options{WorkspaceRoot: "/home/user/project"}) +// defer svc.Shutdown(ctx) type Service struct { server *mcp.Server workspaceRoot string // Root directory for file operations (empty = unrestricted) @@ -32,104 +36,139 @@ type Service struct { wsHub *ws.Hub // WebSocket hub for real-time streaming (optional) wsServer *http.Server // WebSocket HTTP server (optional) wsAddr string // WebSocket server address + wsMu sync.Mutex // Protects wsServer and wsAddr + stdioMode bool // True when running via stdio transport tools []ToolRecord // Parallel tool registry for REST bridge } -// Option configures a Service. -type Option func(*Service) error - -// WithWorkspaceRoot restricts file operations to the given directory. -// All paths are validated to be within this directory. -// An empty string disables the restriction (not recommended). -func WithWorkspaceRoot(root string) Option { - return func(s *Service) error { - if root == "" { - // Explicitly disable restriction - use unsandboxed global - s.workspaceRoot = "" - s.medium = io.Local - return nil - } - // Create sandboxed medium for this workspace - abs, err := filepath.Abs(root) - if err != nil { - return log.E("WithWorkspaceRoot", "invalid workspace root", err) - } - m, err := io.NewSandboxed(abs) - if err != nil { - return log.E("WithWorkspaceRoot", "failed to create workspace medium", err) - } - s.workspaceRoot = abs - s.medium = m - return nil - } +// Options configures a Service. +// +// svc, err := mcp.New(mcp.Options{ +// WorkspaceRoot: "/path/to/project", +// ProcessService: ps, +// Subsystems: []Subsystem{brain, agentic, monitor}, +// }) +type Options struct { + WorkspaceRoot string // Restrict file ops to this directory (empty = cwd) + Unrestricted bool // Disable sandboxing entirely (not recommended) + ProcessService *process.Service // Optional process management + WSHub *ws.Hub // Optional WebSocket hub for real-time streaming + Subsystems []Subsystem // Additional tool groups registered at startup } // New creates a new MCP service with file operations. -// By default, restricts file access to the current working directory. -// Use WithWorkspaceRoot("") to disable restrictions (not recommended). -// Returns an error if initialization fails. -func New(opts ...Option) (*Service, error) { +// +// svc, err := mcp.New(mcp.Options{WorkspaceRoot: "."}) +func New(opts Options) (*Service, error) { impl := &mcp.Implementation{ Name: "core-cli", Version: "0.1.0", } - server := mcp.NewServer(impl, nil) + server := mcp.NewServer(impl, &mcp.ServerOptions{ + Capabilities: &mcp.ServerCapabilities{ + Tools: &mcp.ToolCapabilities{ListChanged: true}, + Logging: &mcp.LoggingCapabilities{}, + Experimental: channelCapability(), + }, + }) + s := &Service{ - server: server, - logger: log.Default(), + server: server, + processService: opts.ProcessService, + wsHub: opts.WSHub, + subsystems: opts.Subsystems, + logger: log.Default(), } - // Default to current working directory with sandboxed medium - cwd, err := os.Getwd() - if err != nil { - return nil, log.E("mcp.New", "failed to get working directory", err) - } - s.workspaceRoot = cwd - m, err := io.NewSandboxed(cwd) - if err != nil { - return nil, log.E("mcp.New", "failed to create sandboxed medium", err) - } - s.medium = m - - // Apply options - for _, opt := range opts { - if err := opt(s); err != nil { - return nil, log.E("mcp.New", "failed to apply option", err) + // Workspace root: unrestricted, explicit root, or default to cwd + if opts.Unrestricted { + s.workspaceRoot = "" + s.medium = io.Local + } else { + root := opts.WorkspaceRoot + if root == "" { + cwd, err := os.Getwd() + if err != nil { + return nil, log.E("mcp.New", "failed to get working directory", err) + } + root = cwd } + abs, err := filepath.Abs(root) + if err != nil { + return nil, log.E("mcp.New", "invalid workspace root", err) + } + m, merr := io.NewSandboxed(abs) + if merr != nil { + return nil, log.E("mcp.New", "failed to create workspace medium", merr) + } + s.workspaceRoot = abs + s.medium = m } s.registerTools(s.server) - // Register subsystem tools. for _, sub := range s.subsystems { sub.RegisterTools(s.server) + if sn, ok := sub.(SubsystemWithNotifier); ok { + sn.SetNotifier(s) + } + // Wire channel callback for subsystems that use func-based notification + type channelWirer interface { + OnChannel(func(ctx context.Context, channel string, data any)) + } + if cw, ok := sub.(channelWirer); ok { + svc := s // capture for closure + cw.OnChannel(func(ctx context.Context, channel string, data any) { + svc.ChannelSend(ctx, channel, data) + }) + } } return s, nil } // Subsystems returns the registered subsystems. +// +// for _, sub := range svc.Subsystems() { +// fmt.Println(sub.Name()) +// } func (s *Service) Subsystems() []Subsystem { return s.subsystems } // SubsystemsSeq returns an iterator over the registered subsystems. +// +// for sub := range svc.SubsystemsSeq() { +// fmt.Println(sub.Name()) +// } func (s *Service) SubsystemsSeq() iter.Seq[Subsystem] { return slices.Values(s.subsystems) } // Tools returns all recorded tool metadata. +// +// for _, t := range svc.Tools() { +// fmt.Printf("%s (%s): %s\n", t.Name, t.Group, t.Description) +// } func (s *Service) Tools() []ToolRecord { return s.tools } // ToolsSeq returns an iterator over all recorded tool metadata. +// +// for rec := range svc.ToolsSeq() { +// fmt.Println(rec.Name) +// } func (s *Service) ToolsSeq() iter.Seq[ToolRecord] { return slices.Values(s.tools) } // Shutdown gracefully shuts down all subsystems that support it. +// +// ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) +// defer cancel() +// if err := svc.Shutdown(ctx); err != nil { log.Fatal(err) } func (s *Service) Shutdown(ctx context.Context) error { for _, sub := range s.subsystems { if sh, ok := sub.(SubsystemWithShutdown); ok { @@ -141,28 +180,21 @@ func (s *Service) Shutdown(ctx context.Context) error { return nil } -// WithProcessService configures the process management service. -func WithProcessService(ps *process.Service) Option { - return func(s *Service) error { - s.processService = ps - return nil - } -} -// WithWSHub configures the WebSocket hub for real-time streaming. -func WithWSHub(hub *ws.Hub) Option { - return func(s *Service) error { - s.wsHub = hub - return nil - } -} - -// WSHub returns the WebSocket hub. +// WSHub returns the WebSocket hub, or nil if not configured. +// +// if hub := svc.WSHub(); hub != nil { +// hub.SendProcessOutput("proc-1", "build complete") +// } func (s *Service) WSHub() *ws.Hub { return s.wsHub } -// ProcessService returns the process service. +// ProcessService returns the process service, or nil if not configured. +// +// if ps := svc.ProcessService(); ps != nil { +// procs := ps.Running() +// } func (s *Service) ProcessService() *process.Service { return s.processService } @@ -226,134 +258,186 @@ func (s *Service) registerTools(server *mcp.Server) { // Tool input/output types for MCP file operations. // ReadFileInput contains parameters for reading a file. +// +// input := ReadFileInput{Path: "src/main.go"} type ReadFileInput struct { - Path string `json:"path"` + Path string `json:"path"` // e.g. "src/main.go" } // ReadFileOutput contains the result of reading a file. +// +// // Returned by the file_read tool: +// // out.Content == "package main\n..." +// // out.Language == "go" +// // out.Path == "src/main.go" type ReadFileOutput struct { - Content string `json:"content"` - Language string `json:"language"` - Path string `json:"path"` + Content string `json:"content"` // e.g. "package main\n..." + Language string `json:"language"` // e.g. "go" + Path string `json:"path"` // e.g. "src/main.go" } // WriteFileInput contains parameters for writing a file. +// +// input := WriteFileInput{Path: "config/app.yaml", Content: "port: 8080\n"} type WriteFileInput struct { - Path string `json:"path"` - Content string `json:"content"` + Path string `json:"path"` // e.g. "config/app.yaml" + Content string `json:"content"` // e.g. "port: 8080\n" } // WriteFileOutput contains the result of writing a file. +// +// // out.Success == true, out.Path == "config/app.yaml" type WriteFileOutput struct { - Success bool `json:"success"` - Path string `json:"path"` + Success bool `json:"success"` // true when the write succeeded + Path string `json:"path"` // e.g. "config/app.yaml" } // ListDirectoryInput contains parameters for listing a directory. +// +// input := ListDirectoryInput{Path: "src/"} type ListDirectoryInput struct { - Path string `json:"path"` + Path string `json:"path"` // e.g. "src/" } // ListDirectoryOutput contains the result of listing a directory. +// +// // out.Path == "src/", len(out.Entries) == 3 type ListDirectoryOutput struct { - Entries []DirectoryEntry `json:"entries"` - Path string `json:"path"` + Entries []DirectoryEntry `json:"entries"` // one entry per file/subdirectory + Path string `json:"path"` // e.g. "src/" } // DirectoryEntry represents a single entry in a directory listing. +// +// // entry.Name == "main.go", entry.IsDir == false, entry.Size == 1024 type DirectoryEntry struct { - Name string `json:"name"` - Path string `json:"path"` - IsDir bool `json:"isDir"` - Size int64 `json:"size"` + Name string `json:"name"` // e.g. "main.go" + Path string `json:"path"` // e.g. "src/main.go" + IsDir bool `json:"isDir"` // true for directories + Size int64 `json:"size"` // file size in bytes } // CreateDirectoryInput contains parameters for creating a directory. +// +// input := CreateDirectoryInput{Path: "src/handlers"} type CreateDirectoryInput struct { - Path string `json:"path"` + Path string `json:"path"` // e.g. "src/handlers" } // CreateDirectoryOutput contains the result of creating a directory. +// +// // out.Success == true, out.Path == "src/handlers" type CreateDirectoryOutput struct { - Success bool `json:"success"` - Path string `json:"path"` + Success bool `json:"success"` // true when creation succeeded + Path string `json:"path"` // e.g. "src/handlers" } // DeleteFileInput contains parameters for deleting a file. +// +// input := DeleteFileInput{Path: "tmp/debug.log"} type DeleteFileInput struct { - Path string `json:"path"` + Path string `json:"path"` // e.g. "tmp/debug.log" } // DeleteFileOutput contains the result of deleting a file. +// +// // out.Success == true, out.Path == "tmp/debug.log" type DeleteFileOutput struct { - Success bool `json:"success"` - Path string `json:"path"` + Success bool `json:"success"` // true when deletion succeeded + Path string `json:"path"` // e.g. "tmp/debug.log" } // RenameFileInput contains parameters for renaming a file. +// +// input := RenameFileInput{OldPath: "pkg/util.go", NewPath: "pkg/helpers.go"} type RenameFileInput struct { - OldPath string `json:"oldPath"` - NewPath string `json:"newPath"` + OldPath string `json:"oldPath"` // e.g. "pkg/util.go" + NewPath string `json:"newPath"` // e.g. "pkg/helpers.go" } // RenameFileOutput contains the result of renaming a file. +// +// // out.Success == true, out.OldPath == "pkg/util.go", out.NewPath == "pkg/helpers.go" type RenameFileOutput struct { - Success bool `json:"success"` - OldPath string `json:"oldPath"` - NewPath string `json:"newPath"` + Success bool `json:"success"` // true when rename succeeded + OldPath string `json:"oldPath"` // e.g. "pkg/util.go" + NewPath string `json:"newPath"` // e.g. "pkg/helpers.go" } // FileExistsInput contains parameters for checking file existence. +// +// input := FileExistsInput{Path: "go.mod"} type FileExistsInput struct { - Path string `json:"path"` + Path string `json:"path"` // e.g. "go.mod" } // FileExistsOutput contains the result of checking file existence. +// +// // out.Exists == true, out.IsDir == false, out.Path == "go.mod" type FileExistsOutput struct { - Exists bool `json:"exists"` - IsDir bool `json:"isDir"` - Path string `json:"path"` + Exists bool `json:"exists"` // true when the path exists + IsDir bool `json:"isDir"` // true when the path is a directory + Path string `json:"path"` // e.g. "go.mod" } // DetectLanguageInput contains parameters for detecting file language. +// +// input := DetectLanguageInput{Path: "cmd/server/main.go"} type DetectLanguageInput struct { - Path string `json:"path"` + Path string `json:"path"` // e.g. "cmd/server/main.go" } // DetectLanguageOutput contains the detected programming language. +// +// // out.Language == "go", out.Path == "cmd/server/main.go" type DetectLanguageOutput struct { - Language string `json:"language"` - Path string `json:"path"` + Language string `json:"language"` // e.g. "go", "typescript", "python" + Path string `json:"path"` // e.g. "cmd/server/main.go" } -// GetSupportedLanguagesInput is an empty struct for the languages query. +// GetSupportedLanguagesInput takes no parameters. +// +// input := GetSupportedLanguagesInput{} type GetSupportedLanguagesInput struct{} // GetSupportedLanguagesOutput contains the list of supported languages. +// +// // len(out.Languages) == 15 +// // out.Languages[0].ID == "typescript" type GetSupportedLanguagesOutput struct { - Languages []LanguageInfo `json:"languages"` + Languages []LanguageInfo `json:"languages"` // all recognised languages } // LanguageInfo describes a supported programming language. +// +// // info.ID == "go", info.Name == "Go", info.Extensions == [".go"] type LanguageInfo struct { - ID string `json:"id"` - Name string `json:"name"` - Extensions []string `json:"extensions"` + ID string `json:"id"` // e.g. "go" + Name string `json:"name"` // e.g. "Go" + Extensions []string `json:"extensions"` // e.g. [".go"] } -// EditDiffInput contains parameters for editing a file via diff. +// EditDiffInput contains parameters for editing a file via string replacement. +// +// input := EditDiffInput{ +// Path: "main.go", +// OldString: "fmt.Println(\"hello\")", +// NewString: "fmt.Println(\"world\")", +// } type EditDiffInput struct { - Path string `json:"path"` - OldString string `json:"old_string"` - NewString string `json:"new_string"` - ReplaceAll bool `json:"replace_all,omitempty"` + Path string `json:"path"` // e.g. "main.go" + OldString string `json:"old_string"` // text to find + NewString string `json:"new_string"` // replacement text + ReplaceAll bool `json:"replace_all,omitempty"` // replace all occurrences (default: first only) } // EditDiffOutput contains the result of a diff-based edit operation. +// +// // out.Success == true, out.Replacements == 1, out.Path == "main.go" type EditDiffOutput struct { - Path string `json:"path"` - Success bool `json:"success"` - Replacements int `json:"replacements"` + Path string `json:"path"` // e.g. "main.go" + Success bool `json:"success"` // true when at least one replacement was made + Replacements int `json:"replacements"` // number of replacements performed } // Tool handlers @@ -561,11 +645,18 @@ func detectLanguageFromPath(path string) string { } } -// Run starts the MCP server. -// Transport selection: -// - MCP_HTTP_ADDR set → Streamable HTTP (with optional MCP_AUTH_TOKEN) -// - MCP_ADDR set → TCP -// - Otherwise → Stdio +// Run starts the MCP server, auto-selecting transport from environment. +// +// // Stdio (default): +// svc.Run(ctx) +// +// // TCP (set MCP_ADDR): +// os.Setenv("MCP_ADDR", "127.0.0.1:9100") +// svc.Run(ctx) +// +// // HTTP (set MCP_HTTP_ADDR): +// os.Setenv("MCP_HTTP_ADDR", "127.0.0.1:9101") +// svc.Run(ctx) func (s *Service) Run(ctx context.Context) error { if httpAddr := os.Getenv("MCP_HTTP_ADDR"); httpAddr != "" { return s.ServeHTTP(ctx, httpAddr) @@ -573,11 +664,15 @@ func (s *Service) Run(ctx context.Context) error { if addr := os.Getenv("MCP_ADDR"); addr != "" { return s.ServeTCP(ctx, addr) } + s.stdioMode = true return s.server.Run(ctx, &mcp.StdioTransport{}) } // Server returns the underlying MCP server for advanced configuration. +// +// server := svc.Server() +// mcp.AddTool(server, &mcp.Tool{Name: "custom_tool"}, handler) func (s *Service) Server() *mcp.Server { return s.server } diff --git a/pkg/mcp/mcp_test.go b/pkg/mcp/mcp_test.go index a1701de..d95beb1 100644 --- a/pkg/mcp/mcp_test.go +++ b/pkg/mcp/mcp_test.go @@ -12,7 +12,7 @@ func TestNew_Good_DefaultWorkspace(t *testing.T) { t.Fatalf("Failed to get working directory: %v", err) } - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -28,7 +28,7 @@ func TestNew_Good_DefaultWorkspace(t *testing.T) { func TestNew_Good_CustomWorkspace(t *testing.T) { tmpDir := t.TempDir() - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -42,7 +42,7 @@ func TestNew_Good_CustomWorkspace(t *testing.T) { } func TestNew_Good_NoRestriction(t *testing.T) { - s, err := New(WithWorkspaceRoot("")) + s, err := New(Options{Unrestricted: true}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -57,7 +57,7 @@ func TestNew_Good_NoRestriction(t *testing.T) { func TestMedium_Good_ReadWrite(t *testing.T) { tmpDir := t.TempDir() - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -87,7 +87,7 @@ func TestMedium_Good_ReadWrite(t *testing.T) { func TestMedium_Good_EnsureDir(t *testing.T) { tmpDir := t.TempDir() - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -110,7 +110,7 @@ func TestMedium_Good_EnsureDir(t *testing.T) { func TestMedium_Good_IsFile(t *testing.T) { tmpDir := t.TempDir() - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -131,7 +131,7 @@ func TestMedium_Good_IsFile(t *testing.T) { func TestSandboxing_Traversal_Sanitized(t *testing.T) { tmpDir := t.TempDir() - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -165,7 +165,7 @@ func TestSandboxing_Symlinks_Blocked(t *testing.T) { t.Skipf("Symlinks not supported: %v", err) } - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } diff --git a/pkg/mcp/notify.go b/pkg/mcp/notify.go new file mode 100644 index 0000000..ff57c03 --- /dev/null +++ b/pkg/mcp/notify.go @@ -0,0 +1,133 @@ +// SPDX-License-Identifier: EUPL-1.2 + +// Notification broadcasting for the MCP service. +// Channel events use the claude/channel experimental capability +// via notifications/claude/channel JSON-RPC notifications. + +package mcp + +import ( + "context" + "encoding/json" + "iter" + "os" + "sync" + + "github.com/modelcontextprotocol/go-sdk/mcp" +) + +// stdoutMu protects stdout writes from concurrent goroutines. +var stdoutMu sync.Mutex + +// SendNotificationToAllClients broadcasts a log-level notification to every +// connected MCP session (stdio, HTTP, TCP, and Unix). +// Errors on individual sessions are logged but do not stop the broadcast. +// +// s.SendNotificationToAllClients(ctx, "info", "monitor", map[string]any{"event": "build complete"}) +func (s *Service) SendNotificationToAllClients(ctx context.Context, level mcp.LoggingLevel, logger string, data any) { + for session := range s.server.Sessions() { + if err := session.Log(ctx, &mcp.LoggingMessageParams{ + Level: level, + Logger: logger, + Data: data, + }); err != nil { + s.logger.Debug("notify: failed to send to session", "session", session.ID(), "error", err) + } + } +} + +// channelNotification is the JSON-RPC notification format for claude/channel. +type channelNotification struct { + JSONRPC string `json:"jsonrpc"` + Method string `json:"method"` + Params channelParams `json:"params"` +} + +type channelParams struct { + Content string `json:"content"` + Meta map[string]string `json:"meta,omitempty"` +} + +// ChannelSend pushes a channel event to all connected clients via +// the notifications/claude/channel JSON-RPC method. +// +// s.ChannelSend(ctx, "agent.complete", map[string]any{"repo": "go-io", "workspace": "go-io-123"}) +// s.ChannelSend(ctx, "build.failed", map[string]any{"repo": "core", "error": "test timeout"}) +func (s *Service) ChannelSend(ctx context.Context, channel string, data any) { + // Marshal the data payload as the content string + contentBytes, err := json.Marshal(data) + if err != nil { + s.logger.Debug("channel: failed to marshal data", "channel", channel, "error", err) + return + } + + notification := channelNotification{ + JSONRPC: "2.0", + Method: "notifications/claude/channel", + Params: channelParams{ + Content: string(contentBytes), + Meta: map[string]string{ + "source": "core-agent", + "channel": channel, + }, + }, + } + + msg, err := json.Marshal(notification) + if err != nil { + s.logger.Debug("channel: failed to marshal notification", "channel", channel, "error", err) + return + } + + // Write directly to stdout (stdio transport) with newline delimiter. + // The official SDK doesn't expose a way to send custom notification methods, + // so we write the JSON-RPC notification directly to the transport. + // Only write when running in stdio mode — HTTP/TCP transports don't use stdout. + if !s.stdioMode { + return + } + stdoutMu.Lock() + os.Stdout.Write(append(msg, '\n')) + stdoutMu.Unlock() +} + +// ChannelSendToSession pushes a channel event to a specific session. +// Falls back to stdout for stdio transport. +// +// s.ChannelSendToSession(ctx, session, "agent.progress", progressData) +func (s *Service) ChannelSendToSession(ctx context.Context, session *mcp.ServerSession, channel string, data any) { + // For now, channel events go to all sessions via stdout + s.ChannelSend(ctx, channel, data) +} + +// Sessions returns an iterator over all connected MCP sessions. +// +// for session := range s.Sessions() { +// s.ChannelSendToSession(ctx, session, "status", data) +// } +func (s *Service) Sessions() iter.Seq[*mcp.ServerSession] { + return s.server.Sessions() +} + +// channelCapability returns the experimental capability descriptor +// for claude/channel, registered during New(). +func channelCapability() map[string]any { + return map[string]any{ + "claude/channel": map[string]any{ + "version": "1", + "description": "Push events into client sessions via named channels", + "channels": []string{ + "agent.complete", + "agent.blocked", + "agent.status", + "build.complete", + "build.failed", + "brain.recall.complete", + "inbox.message", + "process.exit", + "harvest.complete", + "test.result", + }, + }, + } +} diff --git a/pkg/mcp/registry.go b/pkg/mcp/registry.go index 21ae123..363183c 100644 --- a/pkg/mcp/registry.go +++ b/pkg/mcp/registry.go @@ -14,13 +14,23 @@ import ( // RESTHandler handles a tool call from a REST endpoint. // It receives raw JSON input and returns the typed output or an error. +// +// var h RESTHandler = func(ctx context.Context, body []byte) (any, error) { +// var input ReadFileInput +// json.Unmarshal(body, &input) +// return ReadFileOutput{Content: "...", Path: input.Path}, nil +// } type RESTHandler func(ctx context.Context, body []byte) (any, error) // ToolRecord captures metadata about a registered MCP tool. +// +// for _, rec := range svc.Tools() { +// fmt.Printf("tool=%s group=%s desc=%s\n", rec.Name, rec.Group, rec.Description) +// } type ToolRecord struct { - Name string // Tool name, e.g. "file_read" - Description string // Human-readable description - Group string // Subsystem group name, e.g. "files", "rag" + Name string // e.g. "file_read" + Description string // e.g. "Read the contents of a file" + Group string // e.g. "files", "rag", "process" InputSchema map[string]any // JSON Schema from Go struct reflection OutputSchema map[string]any // JSON Schema from Go struct reflection RESTHandler RESTHandler // REST-callable handler created at registration time diff --git a/pkg/mcp/registry_test.go b/pkg/mcp/registry_test.go index 15cdc14..36f5ce6 100644 --- a/pkg/mcp/registry_test.go +++ b/pkg/mcp/registry_test.go @@ -7,7 +7,7 @@ import ( ) func TestToolRegistry_Good_RecordsTools(t *testing.T) { - svc, err := New(WithWorkspaceRoot(t.TempDir())) + svc, err := New(Options{WorkspaceRoot: t.TempDir()}) if err != nil { t.Fatal(err) } @@ -30,7 +30,7 @@ func TestToolRegistry_Good_RecordsTools(t *testing.T) { } func TestToolRegistry_Good_SchemaExtraction(t *testing.T) { - svc, err := New(WithWorkspaceRoot(t.TempDir())) + svc, err := New(Options{WorkspaceRoot: t.TempDir()}) if err != nil { t.Fatal(err) } @@ -61,7 +61,7 @@ func TestToolRegistry_Good_SchemaExtraction(t *testing.T) { } func TestToolRegistry_Good_ToolCount(t *testing.T) { - svc, err := New(WithWorkspaceRoot(t.TempDir())) + svc, err := New(Options{WorkspaceRoot: t.TempDir()}) if err != nil { t.Fatal(err) } @@ -79,7 +79,7 @@ func TestToolRegistry_Good_ToolCount(t *testing.T) { } func TestToolRegistry_Good_GroupAssignment(t *testing.T) { - svc, err := New(WithWorkspaceRoot(t.TempDir())) + svc, err := New(Options{WorkspaceRoot: t.TempDir()}) if err != nil { t.Fatal(err) } @@ -116,7 +116,7 @@ func TestToolRegistry_Good_GroupAssignment(t *testing.T) { } func TestToolRegistry_Good_ToolRecordFields(t *testing.T) { - svc, err := New(WithWorkspaceRoot(t.TempDir())) + svc, err := New(Options{WorkspaceRoot: t.TempDir()}) if err != nil { t.Fatal(err) } diff --git a/pkg/mcp/subsystem.go b/pkg/mcp/subsystem.go index 56bd6f7..67eef39 100644 --- a/pkg/mcp/subsystem.go +++ b/pkg/mcp/subsystem.go @@ -1,3 +1,5 @@ +// SPDX-License-Identifier: EUPL-1.2 + package mcp import ( @@ -8,25 +10,44 @@ import ( // Subsystem registers additional MCP tools at startup. // Implementations should be safe to call concurrently. +// +// type BrainSubsystem struct{} +// func (b *BrainSubsystem) Name() string { return "brain" } +// func (b *BrainSubsystem) RegisterTools(server *mcp.Server) { ... } type Subsystem interface { - // Name returns a human-readable identifier for logging. Name() string - - // RegisterTools adds tools to the MCP server during initialisation. RegisterTools(server *mcp.Server) } // SubsystemWithShutdown extends Subsystem with graceful cleanup. +// +// func (b *BrainSubsystem) Shutdown(ctx context.Context) error { +// return b.client.Close() +// } type SubsystemWithShutdown interface { Subsystem Shutdown(ctx context.Context) error } -// WithSubsystem registers a subsystem whose tools will be added -// after the built-in tools during New(). -func WithSubsystem(sub Subsystem) Option { - return func(s *Service) error { - s.subsystems = append(s.subsystems, sub) - return nil - } +// Notifier pushes events to connected MCP sessions. +// Implemented by *Service. Sub-packages accept this interface +// to avoid circular imports. +// +// notifier.ChannelSend(ctx, "build.complete", data) +type Notifier interface { + ChannelSend(ctx context.Context, channel string, data any) +} + +// Compile-time assertion: *Service implements Notifier. +var _ Notifier = (*Service)(nil) + +// SubsystemWithNotifier extends Subsystem for those that emit channel events. +// SetNotifier is called after New() before any tool calls. +// +// func (m *MonitorSubsystem) SetNotifier(n mcp.Notifier) { +// m.notifier = n +// } +type SubsystemWithNotifier interface { + Subsystem + SetNotifier(n Notifier) } diff --git a/pkg/mcp/subsystem_test.go b/pkg/mcp/subsystem_test.go index 5e823f7..6bda6dc 100644 --- a/pkg/mcp/subsystem_test.go +++ b/pkg/mcp/subsystem_test.go @@ -31,9 +31,9 @@ func (s *shutdownSubsystem) Shutdown(_ context.Context) error { return s.shutdownErr } -func TestWithSubsystem_Good_Registration(t *testing.T) { +func TestSubsystem_Good_Registration(t *testing.T) { sub := &stubSubsystem{name: "test-sub"} - svc, err := New(WithSubsystem(sub)) + svc, err := New(Options{Subsystems: []Subsystem{sub}}) if err != nil { t.Fatalf("New() failed: %v", err) } @@ -46,9 +46,9 @@ func TestWithSubsystem_Good_Registration(t *testing.T) { } } -func TestWithSubsystem_Good_ToolsRegistered(t *testing.T) { +func TestSubsystem_Good_ToolsRegistered(t *testing.T) { sub := &stubSubsystem{name: "tools-sub"} - _, err := New(WithSubsystem(sub)) + _, err := New(Options{Subsystems: []Subsystem{sub}}) if err != nil { t.Fatalf("New() failed: %v", err) } @@ -57,10 +57,10 @@ func TestWithSubsystem_Good_ToolsRegistered(t *testing.T) { } } -func TestWithSubsystem_Good_MultipleSubsystems(t *testing.T) { +func TestSubsystem_Good_MultipleSubsystems(t *testing.T) { sub1 := &stubSubsystem{name: "sub-1"} sub2 := &stubSubsystem{name: "sub-2"} - svc, err := New(WithSubsystem(sub1), WithSubsystem(sub2)) + svc, err := New(Options{Subsystems: []Subsystem{sub1, sub2}}) if err != nil { t.Fatalf("New() failed: %v", err) } @@ -74,7 +74,7 @@ func TestWithSubsystem_Good_MultipleSubsystems(t *testing.T) { func TestSubsystemShutdown_Good(t *testing.T) { sub := &shutdownSubsystem{stubSubsystem: stubSubsystem{name: "shutdown-sub"}} - svc, err := New(WithSubsystem(sub)) + svc, err := New(Options{Subsystems: []Subsystem{sub}}) if err != nil { t.Fatalf("New() failed: %v", err) } @@ -91,7 +91,7 @@ func TestSubsystemShutdown_Bad_Error(t *testing.T) { stubSubsystem: stubSubsystem{name: "fail-sub"}, shutdownErr: context.DeadlineExceeded, } - svc, err := New(WithSubsystem(sub)) + svc, err := New(Options{Subsystems: []Subsystem{sub}}) if err != nil { t.Fatalf("New() failed: %v", err) } @@ -102,9 +102,8 @@ func TestSubsystemShutdown_Bad_Error(t *testing.T) { } func TestSubsystemShutdown_Good_NoShutdownInterface(t *testing.T) { - // A plain Subsystem (without Shutdown) should not cause errors. sub := &stubSubsystem{name: "plain-sub"} - svc, err := New(WithSubsystem(sub)) + svc, err := New(Options{Subsystems: []Subsystem{sub}}) if err != nil { t.Fatalf("New() failed: %v", err) } diff --git a/pkg/mcp/tools_metrics.go b/pkg/mcp/tools_metrics.go index 7fac228..98dd936 100644 --- a/pkg/mcp/tools_metrics.go +++ b/pkg/mcp/tools_metrics.go @@ -19,45 +19,62 @@ const ( ) // MetricsRecordInput contains parameters for recording a metrics event. +// +// input := MetricsRecordInput{ +// Type: "dispatch.complete", +// AgentID: "cladius", +// Repo: "core-php", +// Data: map[string]any{"duration": "4m32s"}, +// } type MetricsRecordInput struct { - Type string `json:"type"` // Event type (required) - AgentID string `json:"agent_id,omitempty"` // Agent identifier - Repo string `json:"repo,omitempty"` // Repository name - Data map[string]any `json:"data,omitempty"` // Additional event data + Type string `json:"type"` // e.g. "dispatch.complete" + AgentID string `json:"agent_id,omitempty"` // e.g. "cladius" + Repo string `json:"repo,omitempty"` // e.g. "core-php" + Data map[string]any `json:"data,omitempty"` // arbitrary key-value data } // MetricsRecordOutput contains the result of recording a metrics event. +// +// // out.Success == true, out.Timestamp == 2026-03-21T14:30:00Z type MetricsRecordOutput struct { - Success bool `json:"success"` - Timestamp time.Time `json:"timestamp"` + Success bool `json:"success"` // true when the event was recorded + Timestamp time.Time `json:"timestamp"` // server-assigned timestamp } // MetricsQueryInput contains parameters for querying metrics. +// +// input := MetricsQueryInput{Since: "24h"} type MetricsQueryInput struct { - Since string `json:"since,omitempty"` // Time range like "7d", "24h", "30m" (default: "7d") + Since string `json:"since,omitempty"` // e.g. "7d", "24h", "30m" (default: "7d") } // MetricsQueryOutput contains the results of a metrics query. +// +// // out.Total == 42, len(out.Events) <= 10 type MetricsQueryOutput struct { - Total int `json:"total"` - ByType []MetricCount `json:"by_type"` - ByRepo []MetricCount `json:"by_repo"` - ByAgent []MetricCount `json:"by_agent"` - Events []MetricEventBrief `json:"events"` // Most recent 10 events + Total int `json:"total"` // total events in range + ByType []MetricCount `json:"by_type"` // counts grouped by event type + ByRepo []MetricCount `json:"by_repo"` // counts grouped by repository + ByAgent []MetricCount `json:"by_agent"` // counts grouped by agent ID + Events []MetricEventBrief `json:"events"` // most recent 10 events } // MetricCount represents a count for a specific key. +// +// // mc.Key == "dispatch.complete", mc.Count == 15 type MetricCount struct { - Key string `json:"key"` - Count int `json:"count"` + Key string `json:"key"` // e.g. "dispatch.complete" or "core-php" + Count int `json:"count"` // number of events matching this key } // MetricEventBrief represents a brief summary of an event. +// +// // ev.Type == "dispatch.complete", ev.AgentID == "cladius", ev.Repo == "core-php" type MetricEventBrief struct { - Type string `json:"type"` - Timestamp time.Time `json:"timestamp"` - AgentID string `json:"agent_id,omitempty"` - Repo string `json:"repo,omitempty"` + Type string `json:"type"` // e.g. "dispatch.complete" + Timestamp time.Time `json:"timestamp"` // when the event occurred + AgentID string `json:"agent_id,omitempty"` // e.g. "cladius" + Repo string `json:"repo,omitempty"` // e.g. "core-php" } // registerMetricsTools adds metrics tools to the MCP server. @@ -132,8 +149,9 @@ func (s *Service) metricsQuery(ctx context.Context, req *mcp.CallToolRequest, in summary := ai.Summary(events) // Build output + total, _ := summary["total"].(int) output := MetricsQueryOutput{ - Total: summary["total"].(int), + Total: total, ByType: convertMetricCounts(summary["by_type"]), ByRepo: convertMetricCounts(summary["by_repo"]), ByAgent: convertMetricCounts(summary["by_agent"]), diff --git a/pkg/mcp/tools_metrics_test.go b/pkg/mcp/tools_metrics_test.go index c34ee6c..ca9decc 100644 --- a/pkg/mcp/tools_metrics_test.go +++ b/pkg/mcp/tools_metrics_test.go @@ -8,7 +8,7 @@ import ( // TestMetricsToolsRegistered_Good verifies that metrics tools are registered with the MCP server. func TestMetricsToolsRegistered_Good(t *testing.T) { // Create a new MCP service - this should register all tools including metrics - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } diff --git a/pkg/mcp/tools_process.go b/pkg/mcp/tools_process.go index 6c41fc7..657f3bb 100644 --- a/pkg/mcp/tools_process.go +++ b/pkg/mcp/tools_process.go @@ -13,92 +13,123 @@ import ( var errIDEmpty = log.E("process", "id cannot be empty", nil) // ProcessStartInput contains parameters for starting a new process. +// +// input := ProcessStartInput{ +// Command: "go", +// Args: []string{"test", "./..."}, +// Dir: "/home/user/project", +// Env: []string{"CGO_ENABLED=0"}, +// } type ProcessStartInput struct { - Command string `json:"command"` // The command to run - Args []string `json:"args,omitempty"` // Command arguments - Dir string `json:"dir,omitempty"` // Working directory - Env []string `json:"env,omitempty"` // Environment variables (KEY=VALUE format) + Command string `json:"command"` // e.g. "go" + Args []string `json:"args,omitempty"` // e.g. ["test", "./..."] + Dir string `json:"dir,omitempty"` // e.g. "/home/user/project" + Env []string `json:"env,omitempty"` // e.g. ["CGO_ENABLED=0"] } // ProcessStartOutput contains the result of starting a process. +// +// // out.ID == "proc-abc123", out.PID == 54321, out.Command == "go" type ProcessStartOutput struct { - ID string `json:"id"` - PID int `json:"pid"` - Command string `json:"command"` - Args []string `json:"args"` - StartedAt time.Time `json:"startedAt"` + ID string `json:"id"` // e.g. "proc-abc123" + PID int `json:"pid"` // OS process ID + Command string `json:"command"` // e.g. "go" + Args []string `json:"args"` // e.g. ["test", "./..."] + StartedAt time.Time `json:"startedAt"` // when the process was started } // ProcessStopInput contains parameters for gracefully stopping a process. +// +// input := ProcessStopInput{ID: "proc-abc123"} type ProcessStopInput struct { - ID string `json:"id"` // Process ID to stop + ID string `json:"id"` // e.g. "proc-abc123" } // ProcessStopOutput contains the result of stopping a process. +// +// // out.Success == true, out.Message == "Process stop signal sent" type ProcessStopOutput struct { - ID string `json:"id"` - Success bool `json:"success"` - Message string `json:"message,omitempty"` + ID string `json:"id"` // e.g. "proc-abc123" + Success bool `json:"success"` // true when stop signal was sent + Message string `json:"message,omitempty"` // e.g. "Process stop signal sent" } // ProcessKillInput contains parameters for force killing a process. +// +// input := ProcessKillInput{ID: "proc-abc123"} type ProcessKillInput struct { - ID string `json:"id"` // Process ID to kill + ID string `json:"id"` // e.g. "proc-abc123" } // ProcessKillOutput contains the result of killing a process. +// +// // out.Success == true, out.Message == "Process killed" type ProcessKillOutput struct { - ID string `json:"id"` - Success bool `json:"success"` - Message string `json:"message,omitempty"` + ID string `json:"id"` // e.g. "proc-abc123" + Success bool `json:"success"` // true when the process was killed + Message string `json:"message,omitempty"` // e.g. "Process killed" } // ProcessListInput contains parameters for listing processes. +// +// input := ProcessListInput{RunningOnly: true} type ProcessListInput struct { - RunningOnly bool `json:"running_only,omitempty"` // If true, only return running processes + RunningOnly bool `json:"running_only,omitempty"` // true to filter to running processes only } // ProcessListOutput contains the list of processes. +// +// // out.Total == 3, len(out.Processes) == 3 type ProcessListOutput struct { - Processes []ProcessInfo `json:"processes"` - Total int `json:"total"` + Processes []ProcessInfo `json:"processes"` // one entry per managed process + Total int `json:"total"` // number of processes returned } -// ProcessInfo represents information about a process. +// ProcessInfo represents information about a managed process. +// +// // info.ID == "proc-abc123", info.Status == "running", info.Command == "go" type ProcessInfo struct { - ID string `json:"id"` - Command string `json:"command"` - Args []string `json:"args"` - Dir string `json:"dir"` - Status string `json:"status"` - PID int `json:"pid"` - ExitCode int `json:"exitCode"` - StartedAt time.Time `json:"startedAt"` - Duration time.Duration `json:"duration"` + ID string `json:"id"` // e.g. "proc-abc123" + Command string `json:"command"` // e.g. "go" + Args []string `json:"args"` // e.g. ["test", "./..."] + Dir string `json:"dir"` // e.g. "/home/user/project" + Status string `json:"status"` // "running", "exited", "killed" + PID int `json:"pid"` // OS process ID + ExitCode int `json:"exitCode"` // 0 on success + StartedAt time.Time `json:"startedAt"` // when the process was started + Duration time.Duration `json:"duration"` // how long the process has run } // ProcessOutputInput contains parameters for getting process output. +// +// input := ProcessOutputInput{ID: "proc-abc123"} type ProcessOutputInput struct { - ID string `json:"id"` // Process ID + ID string `json:"id"` // e.g. "proc-abc123" } // ProcessOutputOutput contains the captured output of a process. +// +// // out.ID == "proc-abc123", out.Output == "PASS\nok core/pkg 1.234s\n" type ProcessOutputOutput struct { - ID string `json:"id"` - Output string `json:"output"` + ID string `json:"id"` // e.g. "proc-abc123" + Output string `json:"output"` // combined stdout/stderr } // ProcessInputInput contains parameters for sending input to a process. +// +// input := ProcessInputInput{ID: "proc-abc123", Input: "yes\n"} type ProcessInputInput struct { - ID string `json:"id"` // Process ID - Input string `json:"input"` // Input to send to stdin + ID string `json:"id"` // e.g. "proc-abc123" + Input string `json:"input"` // e.g. "yes\n" } // ProcessInputOutput contains the result of sending input to a process. +// +// // out.Success == true, out.Message == "Input sent successfully" type ProcessInputOutput struct { - ID string `json:"id"` - Success bool `json:"success"` - Message string `json:"message,omitempty"` + ID string `json:"id"` // e.g. "proc-abc123" + Success bool `json:"success"` // true when input was delivered + Message string `json:"message,omitempty"` // e.g. "Input sent successfully" } // registerProcessTools adds process management tools to the MCP server. @@ -163,13 +194,17 @@ func (s *Service) processStart(ctx context.Context, req *mcp.CallToolRequest, in } info := proc.Info() - return nil, ProcessStartOutput{ + output := ProcessStartOutput{ ID: proc.ID, PID: info.PID, Command: proc.Command, Args: proc.Args, StartedAt: proc.StartedAt, - }, nil + } + s.ChannelSend(ctx, "process.start", map[string]any{ + "id": output.ID, "pid": output.PID, "command": output.Command, + }) + return nil, output, nil } // processStop handles the process_stop tool call. @@ -193,6 +228,7 @@ func (s *Service) processStop(ctx context.Context, req *mcp.CallToolRequest, inp return nil, ProcessStopOutput{}, log.E("processStop", "failed to stop process", err) } + s.ChannelSend(ctx, "process.exit", map[string]any{"id": input.ID, "signal": "stop"}) return nil, ProcessStopOutput{ ID: input.ID, Success: true, @@ -213,6 +249,7 @@ func (s *Service) processKill(ctx context.Context, req *mcp.CallToolRequest, inp return nil, ProcessKillOutput{}, log.E("processKill", "failed to kill process", err) } + s.ChannelSend(ctx, "process.exit", map[string]any{"id": input.ID, "signal": "kill"}) return nil, ProcessKillOutput{ ID: input.ID, Success: true, diff --git a/pkg/mcp/tools_process_ci_test.go b/pkg/mcp/tools_process_ci_test.go index 1aaa9c8..f73314e 100644 --- a/pkg/mcp/tools_process_ci_test.go +++ b/pkg/mcp/tools_process_ci_test.go @@ -1,3 +1,5 @@ +//go:build ci + package mcp import ( @@ -43,7 +45,7 @@ func newTestProcessService(t *testing.T) *process.Service { func newTestMCPWithProcess(t *testing.T) (*Service, *process.Service) { t.Helper() ps := newTestProcessService(t) - s, err := New(WithProcessService(ps)) + s, err := New(Options{ProcessService: ps}) if err != nil { t.Fatalf("Failed to create MCP service: %v", err) } diff --git a/pkg/mcp/tools_process_test.go b/pkg/mcp/tools_process_test.go index 724e2e4..1fb0668 100644 --- a/pkg/mcp/tools_process_test.go +++ b/pkg/mcp/tools_process_test.go @@ -8,7 +8,7 @@ import ( // TestProcessToolsRegistered_Good verifies that process tools are registered when process service is available. func TestProcessToolsRegistered_Good(t *testing.T) { // Create a new MCP service without process service - tools should not be registered - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -279,7 +279,7 @@ func TestProcessInfo_Good(t *testing.T) { func TestWithProcessService_Good(t *testing.T) { // Note: We can't easily create a real process.Service here without Core, // so we just verify the option doesn't panic with nil. - s, err := New(WithProcessService(nil)) + s, err := New(Options{ProcessService: nil}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -288,3 +288,16 @@ func TestWithProcessService_Good(t *testing.T) { t.Error("Expected processService to be nil when passed nil") } } + +// TestRegisterProcessTools_Bad_NilService verifies that tools are not registered when process service is nil. +func TestRegisterProcessTools_Bad_NilService(t *testing.T) { + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + registered := s.registerProcessTools(s.server) + if registered { + t.Error("Expected registerProcessTools to return false when processService is nil") + } +} diff --git a/pkg/mcp/tools_rag.go b/pkg/mcp/tools_rag.go index 96ee716..926009d 100644 --- a/pkg/mcp/tools_rag.go +++ b/pkg/mcp/tools_rag.go @@ -16,61 +16,85 @@ const ( ) // RAGQueryInput contains parameters for querying the RAG vector database. +// +// input := RAGQueryInput{ +// Question: "How do I register a service?", +// Collection: "core-docs", +// TopK: 3, +// } type RAGQueryInput struct { - Question string `json:"question"` // The question or search query - Collection string `json:"collection,omitempty"` // Collection name (default: hostuk-docs) - TopK int `json:"topK,omitempty"` // Number of results to return (default: 5) + Question string `json:"question"` // e.g. "How do I register a service?" + Collection string `json:"collection,omitempty"` // e.g. "core-docs" (default: "hostuk-docs") + TopK int `json:"topK,omitempty"` // e.g. 3 (default: 5) } -// RAGQueryResult represents a single query result. +// RAGQueryResult represents a single query result with relevance score. +// +// // r.Source == "docs/services.md", r.Score == 0.92 type RAGQueryResult struct { - Content string `json:"content"` - Source string `json:"source"` - Section string `json:"section,omitempty"` - Category string `json:"category,omitempty"` - ChunkIndex int `json:"chunkIndex,omitempty"` - Score float32 `json:"score"` + Content string `json:"content"` // matched text chunk + Source string `json:"source"` // e.g. "docs/services.md" + Section string `json:"section,omitempty"` // e.g. "Service Registration" + Category string `json:"category,omitempty"` // e.g. "guide" + ChunkIndex int `json:"chunkIndex,omitempty"` // chunk position within source + Score float32 `json:"score"` // similarity score (0.0-1.0) } // RAGQueryOutput contains the results of a RAG query. +// +// // len(out.Results) == 3, out.Collection == "core-docs" type RAGQueryOutput struct { - Results []RAGQueryResult `json:"results"` - Query string `json:"query"` - Collection string `json:"collection"` - Context string `json:"context"` + Results []RAGQueryResult `json:"results"` // ranked by similarity score + Query string `json:"query"` // the original question + Collection string `json:"collection"` // collection that was searched + Context string `json:"context"` // pre-formatted context string for LLM consumption } // RAGIngestInput contains parameters for ingesting documents into the RAG database. +// +// input := RAGIngestInput{ +// Path: "docs/", +// Collection: "core-docs", +// Recreate: true, +// } type RAGIngestInput struct { - Path string `json:"path"` // File or directory path to ingest - Collection string `json:"collection,omitempty"` // Collection name (default: hostuk-docs) - Recreate bool `json:"recreate,omitempty"` // Whether to recreate the collection + Path string `json:"path"` // e.g. "docs/" or "docs/services.md" + Collection string `json:"collection,omitempty"` // e.g. "core-docs" (default: "hostuk-docs") + Recreate bool `json:"recreate,omitempty"` // true to drop and recreate the collection } // RAGIngestOutput contains the result of a RAG ingest operation. +// +// // out.Success == true, out.Chunks == 42, out.Collection == "core-docs" type RAGIngestOutput struct { - Success bool `json:"success"` - Path string `json:"path"` - Collection string `json:"collection"` - Chunks int `json:"chunks"` - Message string `json:"message,omitempty"` + Success bool `json:"success"` // true when ingest completed + Path string `json:"path"` // e.g. "docs/" + Collection string `json:"collection"` // e.g. "core-docs" + Chunks int `json:"chunks"` // number of chunks ingested + Message string `json:"message,omitempty"` // human-readable summary } // RAGCollectionsInput contains parameters for listing collections. +// +// input := RAGCollectionsInput{ShowStats: true} type RAGCollectionsInput struct { - ShowStats bool `json:"show_stats,omitempty"` // Include collection stats (point count, status) + ShowStats bool `json:"show_stats,omitempty"` // true to include point counts and status } -// CollectionInfo contains information about a collection. +// CollectionInfo contains information about a Qdrant collection. +// +// // ci.Name == "core-docs", ci.PointsCount == 1500, ci.Status == "green" type CollectionInfo struct { - Name string `json:"name"` - PointsCount uint64 `json:"points_count"` - Status string `json:"status"` + Name string `json:"name"` // e.g. "core-docs" + PointsCount uint64 `json:"points_count"` // number of vectors stored + Status string `json:"status"` // e.g. "green" } // RAGCollectionsOutput contains the list of available collections. +// +// // len(out.Collections) == 2 type RAGCollectionsOutput struct { - Collections []CollectionInfo `json:"collections"` + Collections []CollectionInfo `json:"collections"` // all Qdrant collections } // registerRAGTools adds RAG tools to the MCP server. diff --git a/pkg/mcp/tools_rag_ci_test.go b/pkg/mcp/tools_rag_ci_test.go index fb7d853..4535a8e 100644 --- a/pkg/mcp/tools_rag_ci_test.go +++ b/pkg/mcp/tools_rag_ci_test.go @@ -15,7 +15,7 @@ import ( // TestRagQuery_Bad_EmptyQuestion verifies empty question returns error. func TestRagQuery_Bad_EmptyQuestion(t *testing.T) { - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -35,7 +35,7 @@ func TestRagQuery_Bad_EmptyQuestion(t *testing.T) { // zero Collection/TopK should have defaults applied. We cannot verify the actual // query (needs live Qdrant), but we can verify it gets past validation. func TestRagQuery_Good_DefaultsApplied(t *testing.T) { - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -57,7 +57,7 @@ func TestRagQuery_Good_DefaultsApplied(t *testing.T) { // TestRagIngest_Bad_EmptyPath verifies empty path returns error. func TestRagIngest_Bad_EmptyPath(t *testing.T) { - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -74,7 +74,7 @@ func TestRagIngest_Bad_EmptyPath(t *testing.T) { // TestRagIngest_Bad_NonexistentPath verifies nonexistent path returns error. func TestRagIngest_Bad_NonexistentPath(t *testing.T) { - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -90,7 +90,7 @@ func TestRagIngest_Bad_NonexistentPath(t *testing.T) { // TestRagIngest_Good_DefaultCollection verifies the default collection is applied. func TestRagIngest_Good_DefaultCollection(t *testing.T) { - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -114,7 +114,7 @@ func TestRagIngest_Good_DefaultCollection(t *testing.T) { // TestRagCollections_Bad_NoQdrant verifies graceful error when Qdrant is not available. func TestRagCollections_Bad_NoQdrant(t *testing.T) { - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } diff --git a/pkg/mcp/tools_rag_test.go b/pkg/mcp/tools_rag_test.go index 1c344f3..281dbf0 100644 --- a/pkg/mcp/tools_rag_test.go +++ b/pkg/mcp/tools_rag_test.go @@ -7,7 +7,7 @@ import ( // TestRAGToolsRegistered_Good verifies that RAG tools are registered with the MCP server. func TestRAGToolsRegistered_Good(t *testing.T) { // Create a new MCP service - this should register all tools including RAG - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } diff --git a/pkg/mcp/tools_webview.go b/pkg/mcp/tools_webview.go index 7bc1e32..7a85393 100644 --- a/pkg/mcp/tools_webview.go +++ b/pkg/mcp/tools_webview.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "fmt" + "sync" "time" "forge.lthn.ai/core/go-log" @@ -11,6 +12,9 @@ import ( "github.com/modelcontextprotocol/go-sdk/mcp" ) +// webviewMu protects webviewInstance from concurrent access. +var webviewMu sync.Mutex + // webviewInstance holds the current webview connection. // This is managed by the MCP service. var webviewInstance *webview.Webview @@ -22,133 +26,177 @@ var ( ) // WebviewConnectInput contains parameters for connecting to Chrome DevTools. +// +// input := WebviewConnectInput{DebugURL: "http://localhost:9222", Timeout: 10} type WebviewConnectInput struct { - DebugURL string `json:"debug_url"` // Chrome DevTools URL (e.g., http://localhost:9222) - Timeout int `json:"timeout,omitempty"` // Default timeout in seconds (default: 30) + DebugURL string `json:"debug_url"` // e.g. "http://localhost:9222" + Timeout int `json:"timeout,omitempty"` // seconds (default: 30) } // WebviewConnectOutput contains the result of connecting to Chrome. +// +// // out.Success == true, out.Message == "Connected to Chrome DevTools at http://localhost:9222" type WebviewConnectOutput struct { - Success bool `json:"success"` - Message string `json:"message,omitempty"` + Success bool `json:"success"` // true when connection established + Message string `json:"message,omitempty"` // connection status } // WebviewNavigateInput contains parameters for navigating to a URL. +// +// input := WebviewNavigateInput{URL: "https://lthn.ai/dashboard"} type WebviewNavigateInput struct { - URL string `json:"url"` // URL to navigate to + URL string `json:"url"` // e.g. "https://lthn.ai/dashboard" } // WebviewNavigateOutput contains the result of navigation. +// +// // out.Success == true, out.URL == "https://lthn.ai/dashboard" type WebviewNavigateOutput struct { - Success bool `json:"success"` - URL string `json:"url"` + Success bool `json:"success"` // true when navigation completed + URL string `json:"url"` // the URL navigated to } // WebviewClickInput contains parameters for clicking an element. +// +// input := WebviewClickInput{Selector: "button.submit"} type WebviewClickInput struct { - Selector string `json:"selector"` // CSS selector + Selector string `json:"selector"` // e.g. "button.submit" } // WebviewClickOutput contains the result of a click action. +// +// // out.Success == true type WebviewClickOutput struct { - Success bool `json:"success"` + Success bool `json:"success"` // true when the click was performed } -// WebviewTypeInput contains parameters for typing text. +// WebviewTypeInput contains parameters for typing text into a form element. +// +// input := WebviewTypeInput{Selector: "input#email", Text: "user@example.com"} type WebviewTypeInput struct { - Selector string `json:"selector"` // CSS selector - Text string `json:"text"` // Text to type + Selector string `json:"selector"` // e.g. "input#email" + Text string `json:"text"` // e.g. "user@example.com" } // WebviewTypeOutput contains the result of a type action. +// +// // out.Success == true type WebviewTypeOutput struct { - Success bool `json:"success"` + Success bool `json:"success"` // true when text was typed } -// WebviewQueryInput contains parameters for querying an element. +// WebviewQueryInput contains parameters for querying DOM elements. +// +// input := WebviewQueryInput{Selector: "div.card", All: true} type WebviewQueryInput struct { - Selector string `json:"selector"` // CSS selector - All bool `json:"all,omitempty"` // If true, return all matching elements + Selector string `json:"selector"` // e.g. "div.card" + All bool `json:"all,omitempty"` // true to return all matches (default: first only) } -// WebviewQueryOutput contains the result of a query. +// WebviewQueryOutput contains the result of a DOM query. +// +// // out.Found == true, out.Count == 3, len(out.Elements) == 3 type WebviewQueryOutput struct { - Found bool `json:"found"` - Count int `json:"count"` - Elements []WebviewElementInfo `json:"elements,omitempty"` + Found bool `json:"found"` // true when at least one element matched + Count int `json:"count"` // number of matches + Elements []WebviewElementInfo `json:"elements,omitempty"` // matched elements } // WebviewElementInfo represents information about a DOM element. +// +// // el.TagName == "div", el.Attributes["class"] == "card active" type WebviewElementInfo struct { - NodeID int `json:"nodeId"` - TagName string `json:"tagName"` - Attributes map[string]string `json:"attributes,omitempty"` - BoundingBox *webview.BoundingBox `json:"boundingBox,omitempty"` + NodeID int `json:"nodeId"` // CDP node identifier + TagName string `json:"tagName"` // e.g. "div", "button" + Attributes map[string]string `json:"attributes,omitempty"` // e.g. {"class": "card", "id": "main"} + BoundingBox *webview.BoundingBox `json:"boundingBox,omitempty"` // viewport coordinates } // WebviewConsoleInput contains parameters for getting console output. +// +// input := WebviewConsoleInput{Clear: true} type WebviewConsoleInput struct { - Clear bool `json:"clear,omitempty"` // If true, clear console after getting messages + Clear bool `json:"clear,omitempty"` // true to clear the buffer after reading } // WebviewConsoleOutput contains console messages. +// +// // out.Count == 5, out.Messages[0].Type == "log" type WebviewConsoleOutput struct { - Messages []WebviewConsoleMessage `json:"messages"` - Count int `json:"count"` + Messages []WebviewConsoleMessage `json:"messages"` // captured console entries + Count int `json:"count"` // number of messages } -// WebviewConsoleMessage represents a console message. +// WebviewConsoleMessage represents a single browser console entry. +// +// // msg.Type == "log", msg.Text == "App loaded" type WebviewConsoleMessage struct { - Type string `json:"type"` - Text string `json:"text"` - Timestamp string `json:"timestamp"` - URL string `json:"url,omitempty"` - Line int `json:"line,omitempty"` + Type string `json:"type"` // e.g. "log", "warn", "error" + Text string `json:"text"` // e.g. "App loaded" + Timestamp string `json:"timestamp"` // RFC3339 formatted + URL string `json:"url,omitempty"` // source file URL + Line int `json:"line,omitempty"` // source line number } // WebviewEvalInput contains parameters for evaluating JavaScript. +// +// input := WebviewEvalInput{Script: "document.title"} type WebviewEvalInput struct { - Script string `json:"script"` // JavaScript to evaluate + Script string `json:"script"` // e.g. "document.title" } // WebviewEvalOutput contains the result of JavaScript evaluation. +// +// // out.Success == true, out.Result == "Dashboard - Host UK" type WebviewEvalOutput struct { - Success bool `json:"success"` - Result any `json:"result,omitempty"` - Error string `json:"error,omitempty"` + Success bool `json:"success"` // true when script executed without error + Result any `json:"result,omitempty"` // return value of the script + Error string `json:"error,omitempty"` // JS error message if execution failed } // WebviewScreenshotInput contains parameters for taking a screenshot. +// +// input := WebviewScreenshotInput{Format: "png"} type WebviewScreenshotInput struct { - Format string `json:"format,omitempty"` // "png" or "jpeg" (default: png) + Format string `json:"format,omitempty"` // "png" or "jpeg" (default: "png") } // WebviewScreenshotOutput contains the screenshot data. +// +// // out.Success == true, out.Format == "png", len(out.Data) > 0 type WebviewScreenshotOutput struct { - Success bool `json:"success"` - Data string `json:"data"` // Base64 encoded image - Format string `json:"format"` + Success bool `json:"success"` // true when screenshot was captured + Data string `json:"data"` // base64-encoded image bytes + Format string `json:"format"` // "png" or "jpeg" } -// WebviewWaitInput contains parameters for waiting operations. +// WebviewWaitInput contains parameters for waiting for an element to appear. +// +// input := WebviewWaitInput{Selector: "div.loaded", Timeout: 10} type WebviewWaitInput struct { - Selector string `json:"selector,omitempty"` // Wait for selector - Timeout int `json:"timeout,omitempty"` // Timeout in seconds + Selector string `json:"selector,omitempty"` // e.g. "div.loaded" + Timeout int `json:"timeout,omitempty"` // seconds to wait before timing out } -// WebviewWaitOutput contains the result of waiting. +// WebviewWaitOutput contains the result of waiting for an element. +// +// // out.Success == true, out.Message == "Element found: div.loaded" type WebviewWaitOutput struct { - Success bool `json:"success"` - Message string `json:"message,omitempty"` + Success bool `json:"success"` // true when element appeared + Message string `json:"message,omitempty"` // e.g. "Element found: div.loaded" } -// WebviewDisconnectInput contains parameters for disconnecting. +// WebviewDisconnectInput takes no parameters. +// +// input := WebviewDisconnectInput{} type WebviewDisconnectInput struct{} // WebviewDisconnectOutput contains the result of disconnecting. +// +// // out.Success == true, out.Message == "Disconnected from Chrome DevTools" type WebviewDisconnectOutput struct { - Success bool `json:"success"` - Message string `json:"message,omitempty"` + Success bool `json:"success"` // true when disconnection completed + Message string `json:"message,omitempty"` // e.g. "Disconnected from Chrome DevTools" } // registerWebviewTools adds webview tools to the MCP server. @@ -206,6 +254,9 @@ func (s *Service) registerWebviewTools(server *mcp.Server) { // webviewConnect handles the webview_connect tool call. func (s *Service) webviewConnect(ctx context.Context, req *mcp.CallToolRequest, input WebviewConnectInput) (*mcp.CallToolResult, WebviewConnectOutput, error) { + webviewMu.Lock() + defer webviewMu.Unlock() + s.logger.Security("MCP tool execution", "tool", "webview_connect", "debug_url", input.DebugURL, "user", log.Username()) if input.DebugURL == "" { @@ -244,6 +295,9 @@ func (s *Service) webviewConnect(ctx context.Context, req *mcp.CallToolRequest, // webviewDisconnect handles the webview_disconnect tool call. func (s *Service) webviewDisconnect(ctx context.Context, req *mcp.CallToolRequest, input WebviewDisconnectInput) (*mcp.CallToolResult, WebviewDisconnectOutput, error) { + webviewMu.Lock() + defer webviewMu.Unlock() + s.logger.Info("MCP tool execution", "tool", "webview_disconnect", "user", log.Username()) if webviewInstance == nil { @@ -268,6 +322,9 @@ func (s *Service) webviewDisconnect(ctx context.Context, req *mcp.CallToolReques // webviewNavigate handles the webview_navigate tool call. func (s *Service) webviewNavigate(ctx context.Context, req *mcp.CallToolRequest, input WebviewNavigateInput) (*mcp.CallToolResult, WebviewNavigateOutput, error) { + webviewMu.Lock() + defer webviewMu.Unlock() + s.logger.Info("MCP tool execution", "tool", "webview_navigate", "url", input.URL, "user", log.Username()) if webviewInstance == nil { @@ -291,6 +348,9 @@ func (s *Service) webviewNavigate(ctx context.Context, req *mcp.CallToolRequest, // webviewClick handles the webview_click tool call. func (s *Service) webviewClick(ctx context.Context, req *mcp.CallToolRequest, input WebviewClickInput) (*mcp.CallToolResult, WebviewClickOutput, error) { + webviewMu.Lock() + defer webviewMu.Unlock() + s.logger.Info("MCP tool execution", "tool", "webview_click", "selector", input.Selector, "user", log.Username()) if webviewInstance == nil { @@ -311,6 +371,9 @@ func (s *Service) webviewClick(ctx context.Context, req *mcp.CallToolRequest, in // webviewType handles the webview_type tool call. func (s *Service) webviewType(ctx context.Context, req *mcp.CallToolRequest, input WebviewTypeInput) (*mcp.CallToolResult, WebviewTypeOutput, error) { + webviewMu.Lock() + defer webviewMu.Unlock() + s.logger.Info("MCP tool execution", "tool", "webview_type", "selector", input.Selector, "user", log.Username()) if webviewInstance == nil { @@ -331,6 +394,9 @@ func (s *Service) webviewType(ctx context.Context, req *mcp.CallToolRequest, inp // webviewQuery handles the webview_query tool call. func (s *Service) webviewQuery(ctx context.Context, req *mcp.CallToolRequest, input WebviewQueryInput) (*mcp.CallToolResult, WebviewQueryOutput, error) { + webviewMu.Lock() + defer webviewMu.Unlock() + s.logger.Info("MCP tool execution", "tool", "webview_query", "selector", input.Selector, "all", input.All, "user", log.Username()) if webviewInstance == nil { @@ -389,6 +455,9 @@ func (s *Service) webviewQuery(ctx context.Context, req *mcp.CallToolRequest, in // webviewConsole handles the webview_console tool call. func (s *Service) webviewConsole(ctx context.Context, req *mcp.CallToolRequest, input WebviewConsoleInput) (*mcp.CallToolResult, WebviewConsoleOutput, error) { + webviewMu.Lock() + defer webviewMu.Unlock() + s.logger.Info("MCP tool execution", "tool", "webview_console", "clear", input.Clear, "user", log.Username()) if webviewInstance == nil { @@ -421,6 +490,9 @@ func (s *Service) webviewConsole(ctx context.Context, req *mcp.CallToolRequest, // webviewEval handles the webview_eval tool call. func (s *Service) webviewEval(ctx context.Context, req *mcp.CallToolRequest, input WebviewEvalInput) (*mcp.CallToolResult, WebviewEvalOutput, error) { + webviewMu.Lock() + defer webviewMu.Unlock() + s.logger.Security("MCP tool execution", "tool", "webview_eval", "user", log.Username()) if webviewInstance == nil { @@ -448,6 +520,9 @@ func (s *Service) webviewEval(ctx context.Context, req *mcp.CallToolRequest, inp // webviewScreenshot handles the webview_screenshot tool call. func (s *Service) webviewScreenshot(ctx context.Context, req *mcp.CallToolRequest, input WebviewScreenshotInput) (*mcp.CallToolResult, WebviewScreenshotOutput, error) { + webviewMu.Lock() + defer webviewMu.Unlock() + s.logger.Info("MCP tool execution", "tool", "webview_screenshot", "format", input.Format, "user", log.Username()) if webviewInstance == nil { @@ -474,6 +549,9 @@ func (s *Service) webviewScreenshot(ctx context.Context, req *mcp.CallToolReques // webviewWait handles the webview_wait tool call. func (s *Service) webviewWait(ctx context.Context, req *mcp.CallToolRequest, input WebviewWaitInput) (*mcp.CallToolResult, WebviewWaitOutput, error) { + webviewMu.Lock() + defer webviewMu.Unlock() + s.logger.Info("MCP tool execution", "tool", "webview_wait", "selector", input.Selector, "timeout", input.Timeout, "user", log.Username()) if webviewInstance == nil { diff --git a/pkg/mcp/tools_webview_test.go b/pkg/mcp/tools_webview_test.go index abb00fa..1849430 100644 --- a/pkg/mcp/tools_webview_test.go +++ b/pkg/mcp/tools_webview_test.go @@ -22,7 +22,7 @@ func skipIfShort(t *testing.T) { // TestWebviewToolsRegistered_Good verifies that webview tools are registered with the MCP server. func TestWebviewToolsRegistered_Good(t *testing.T) { // Create a new MCP service - this should register all tools including webview - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -48,7 +48,7 @@ func TestWebviewToolHandlers_RequiresChrome(t *testing.T) { // This test verifies that webview tool handlers correctly reject // calls when not connected to Chrome. tmpDir := t.TempDir() - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -450,3 +450,151 @@ func TestWebviewWaitOutput_Good(t *testing.T) { t.Error("Expected message to be set") } } + +// --- Handler tests beyond nil-guard --- + +// setStubWebview injects a zero-value Webview stub so handler validation +// logic beyond the nil-guard can be exercised without a running Chrome. +// The previous value is restored via t.Cleanup. +func setStubWebview(t *testing.T) { + t.Helper() + webviewMu.Lock() + old := webviewInstance + webviewInstance = &webview.Webview{} + webviewMu.Unlock() + t.Cleanup(func() { + webviewMu.Lock() + webviewInstance = old + webviewMu.Unlock() + }) +} + +// TestWebviewDisconnect_Good_NoConnection verifies disconnect succeeds when not connected. +func TestWebviewDisconnect_Good_NoConnection(t *testing.T) { + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + ctx := t.Context() + _, out, err := s.webviewDisconnect(ctx, nil, WebviewDisconnectInput{}) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + if !out.Success { + t.Error("Expected success to be true") + } + if out.Message != "No active connection" { + t.Errorf("Expected message 'No active connection', got %q", out.Message) + } +} + +// TestWebviewConnect_Bad_EmptyURL verifies connect rejects an empty debug URL. +func TestWebviewConnect_Bad_EmptyURL(t *testing.T) { + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + ctx := t.Context() + _, _, err = s.webviewConnect(ctx, nil, WebviewConnectInput{DebugURL: ""}) + if err == nil { + t.Error("Expected error for empty debug URL, got nil") + } +} + +// TestWebviewNavigate_Bad_EmptyURL verifies navigate rejects an empty URL. +func TestWebviewNavigate_Bad_EmptyURL(t *testing.T) { + setStubWebview(t) + + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + ctx := t.Context() + _, _, err = s.webviewNavigate(ctx, nil, WebviewNavigateInput{URL: ""}) + if err == nil { + t.Error("Expected error for empty URL, got nil") + } +} + +// TestWebviewClick_Bad_EmptySelector verifies click rejects an empty selector. +func TestWebviewClick_Bad_EmptySelector(t *testing.T) { + setStubWebview(t) + + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + ctx := t.Context() + _, _, err = s.webviewClick(ctx, nil, WebviewClickInput{Selector: ""}) + if err == nil { + t.Error("Expected error for empty selector, got nil") + } +} + +// TestWebviewType_Bad_EmptySelector verifies type rejects an empty selector. +func TestWebviewType_Bad_EmptySelector(t *testing.T) { + setStubWebview(t) + + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + ctx := t.Context() + _, _, err = s.webviewType(ctx, nil, WebviewTypeInput{Selector: "", Text: "test"}) + if err == nil { + t.Error("Expected error for empty selector, got nil") + } +} + +// TestWebviewQuery_Bad_EmptySelector verifies query rejects an empty selector. +func TestWebviewQuery_Bad_EmptySelector(t *testing.T) { + setStubWebview(t) + + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + ctx := t.Context() + _, _, err = s.webviewQuery(ctx, nil, WebviewQueryInput{Selector: ""}) + if err == nil { + t.Error("Expected error for empty selector, got nil") + } +} + +// TestWebviewEval_Bad_EmptyScript verifies eval rejects an empty script. +func TestWebviewEval_Bad_EmptyScript(t *testing.T) { + setStubWebview(t) + + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + ctx := t.Context() + _, _, err = s.webviewEval(ctx, nil, WebviewEvalInput{Script: ""}) + if err == nil { + t.Error("Expected error for empty script, got nil") + } +} + +// TestWebviewWait_Bad_EmptySelector verifies wait rejects an empty selector. +func TestWebviewWait_Bad_EmptySelector(t *testing.T) { + setStubWebview(t) + + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + ctx := t.Context() + _, _, err = s.webviewWait(ctx, nil, WebviewWaitInput{Selector: ""}) + if err == nil { + t.Error("Expected error for empty selector, got nil") + } +} diff --git a/pkg/mcp/tools_ws.go b/pkg/mcp/tools_ws.go index 254ee3b..d5aae2a 100644 --- a/pkg/mcp/tools_ws.go +++ b/pkg/mcp/tools_ws.go @@ -12,24 +12,32 @@ import ( ) // WSStartInput contains parameters for starting the WebSocket server. +// +// input := WSStartInput{Addr: ":9090"} type WSStartInput struct { - Addr string `json:"addr,omitempty"` // Address to listen on (default: ":8080") + Addr string `json:"addr,omitempty"` // e.g. ":9090" (default: ":8080") } // WSStartOutput contains the result of starting the WebSocket server. +// +// // out.Success == true, out.Addr == "127.0.0.1:9090" type WSStartOutput struct { - Success bool `json:"success"` - Addr string `json:"addr"` - Message string `json:"message,omitempty"` + Success bool `json:"success"` // true when server started + Addr string `json:"addr"` // actual listening address + Message string `json:"message,omitempty"` // e.g. "WebSocket server started at ws://127.0.0.1:9090/ws" } -// WSInfoInput contains parameters for getting WebSocket hub info. +// WSInfoInput takes no parameters. +// +// input := WSInfoInput{} type WSInfoInput struct{} // WSInfoOutput contains WebSocket hub statistics. +// +// // out.Clients == 3, out.Channels == 2 type WSInfoOutput struct { - Clients int `json:"clients"` - Channels int `json:"channels"` + Clients int `json:"clients"` // number of connected WebSocket clients + Channels int `json:"channels"` // number of active channels } // registerWSTools adds WebSocket tools to the MCP server. @@ -61,6 +69,9 @@ func (s *Service) wsStart(ctx context.Context, req *mcp.CallToolRequest, input W s.logger.Security("MCP tool execution", "tool", "ws_start", "addr", addr, "user", log.Username()) + s.wsMu.Lock() + defer s.wsMu.Unlock() + // Check if server is already running if s.wsServer != nil { return nil, WSStartOutput{ @@ -116,18 +127,25 @@ func (s *Service) wsInfo(ctx context.Context, req *mcp.CallToolRequest, input WS }, nil } -// ProcessEventCallback is a callback function for process events. -// It can be registered with the process service to forward events to WebSocket. +// ProcessEventCallback forwards process lifecycle events to WebSocket clients. +// +// cb := NewProcessEventCallback(hub) +// cb.OnProcessOutput("proc-abc123", "build complete\n") +// cb.OnProcessStatus("proc-abc123", "exited", 0) type ProcessEventCallback struct { hub *ws.Hub } // NewProcessEventCallback creates a callback that forwards process events to WebSocket. +// +// cb := NewProcessEventCallback(hub) func NewProcessEventCallback(hub *ws.Hub) *ProcessEventCallback { return &ProcessEventCallback{hub: hub} } // OnProcessOutput forwards process output to WebSocket subscribers. +// +// cb.OnProcessOutput("proc-abc123", "PASS\n") func (c *ProcessEventCallback) OnProcessOutput(processID string, line string) { if c.hub != nil { _ = c.hub.SendProcessOutput(processID, line) @@ -135,6 +153,8 @@ func (c *ProcessEventCallback) OnProcessOutput(processID string, line string) { } // OnProcessStatus forwards process status changes to WebSocket subscribers. +// +// cb.OnProcessStatus("proc-abc123", "exited", 0) func (c *ProcessEventCallback) OnProcessStatus(processID string, status string, exitCode int) { if c.hub != nil { _ = c.hub.SendProcessStatus(processID, status, exitCode) diff --git a/pkg/mcp/tools_ws_test.go b/pkg/mcp/tools_ws_test.go index 2ffaa51..cde21fd 100644 --- a/pkg/mcp/tools_ws_test.go +++ b/pkg/mcp/tools_ws_test.go @@ -9,7 +9,7 @@ import ( // TestWSToolsRegistered_Good verifies that WebSocket tools are registered when hub is available. func TestWSToolsRegistered_Good(t *testing.T) { // Create a new MCP service without ws hub - tools should not be registered - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -87,7 +87,7 @@ func TestWSInfoOutput_Good(t *testing.T) { func TestWithWSHub_Good(t *testing.T) { hub := ws.NewHub() - s, err := New(WithWSHub(hub)) + s, err := New(Options{WSHub: hub}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -99,7 +99,7 @@ func TestWithWSHub_Good(t *testing.T) { // TestWithWSHub_Nil verifies the WithWSHub option with nil. func TestWithWSHub_Nil(t *testing.T) { - s, err := New(WithWSHub(nil)) + s, err := New(Options{WSHub: nil}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -139,7 +139,7 @@ func TestProcessEventCallback_NilHub(t *testing.T) { // TestServiceWSHub_Good verifies the WSHub getter method. func TestServiceWSHub_Good(t *testing.T) { hub := ws.NewHub() - s, err := New(WithWSHub(hub)) + s, err := New(Options{WSHub: hub}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -151,7 +151,7 @@ func TestServiceWSHub_Good(t *testing.T) { // TestServiceWSHub_Nil verifies the WSHub getter returns nil when not configured. func TestServiceWSHub_Nil(t *testing.T) { - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -163,7 +163,7 @@ func TestServiceWSHub_Nil(t *testing.T) { // TestServiceProcessService_Nil verifies the ProcessService getter returns nil when not configured. func TestServiceProcessService_Nil(t *testing.T) { - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } diff --git a/pkg/mcp/transport_e2e_test.go b/pkg/mcp/transport_e2e_test.go index 1a9a8d0..167387c 100644 --- a/pkg/mcp/transport_e2e_test.go +++ b/pkg/mcp/transport_e2e_test.go @@ -2,6 +2,7 @@ package mcp import ( "bufio" + "context" "encoding/json" "fmt" "net" @@ -10,8 +11,6 @@ import ( "strings" "testing" "time" - - "context" ) // jsonRPCRequest builds a raw JSON-RPC 2.0 request string with newline delimiter. @@ -87,7 +86,7 @@ func TestTCPTransport_E2E_FullRoundTrip(t *testing.T) { t.Fatalf("Failed to create test file: %v", err) } - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -148,20 +147,20 @@ func TestTCPTransport_E2E_FullRoundTrip(t *testing.T) { scanner := bufio.NewScanner(conn) scanner.Buffer(make([]byte, 64*1024), 10*1024*1024) - // Step 1: Send initialize request + // Step 1: Send initialise request initReq := jsonRPCRequest(1, "initialize", map[string]any{ "protocolVersion": "2024-11-05", "capabilities": map[string]any{}, "clientInfo": map[string]any{"name": "TestClient", "version": "1.0.0"}, }) if _, err := conn.Write([]byte(initReq)); err != nil { - t.Fatalf("Failed to send initialize: %v", err) + t.Fatalf("Failed to send initialise: %v", err) } - // Read initialize response + // Read initialise response initResp := readJSONRPCResponse(t, scanner, conn) if initResp["error"] != nil { - t.Fatalf("Initialize returned error: %v", initResp["error"]) + t.Fatalf("Initialise returned error: %v", initResp["error"]) } result, ok := initResp["result"].(map[string]any) if !ok { @@ -251,7 +250,7 @@ func TestTCPTransport_E2E_FullRoundTrip(t *testing.T) { func TestTCPTransport_E2E_FileWrite(t *testing.T) { tmpDir := t.TempDir() - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -291,7 +290,7 @@ func TestTCPTransport_E2E_FileWrite(t *testing.T) { scanner := bufio.NewScanner(conn) scanner.Buffer(make([]byte, 64*1024), 10*1024*1024) - // Initialize handshake + // Initialise handshake conn.Write([]byte(jsonRPCRequest(1, "initialize", map[string]any{ "protocolVersion": "2024-11-05", "capabilities": map[string]any{}, @@ -344,7 +343,7 @@ func TestUnixTransport_E2E_FullRoundTrip(t *testing.T) { t.Fatalf("Failed to create test file: %v", err) } - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -379,7 +378,7 @@ func TestUnixTransport_E2E_FullRoundTrip(t *testing.T) { scanner := bufio.NewScanner(conn) scanner.Buffer(make([]byte, 64*1024), 10*1024*1024) - // Step 1: Initialize + // Step 1: Initialise conn.Write([]byte(jsonRPCRequest(1, "initialize", map[string]any{ "protocolVersion": "2024-11-05", "capabilities": map[string]any{}, @@ -387,10 +386,10 @@ func TestUnixTransport_E2E_FullRoundTrip(t *testing.T) { }))) initResp := readJSONRPCResponse(t, scanner, conn) if initResp["error"] != nil { - t.Fatalf("Initialize returned error: %v", initResp["error"]) + t.Fatalf("Initialise returned error: %v", initResp["error"]) } - // Step 2: Send initialized notification + // Step 2: Send initialised notification conn.Write([]byte(jsonRPCNotification("notifications/initialized"))) // Step 3: tools/list @@ -455,7 +454,7 @@ func TestUnixTransport_E2E_DirList(t *testing.T) { os.WriteFile(filepath.Join(tmpDir, "file1.txt"), []byte("one"), 0644) os.WriteFile(filepath.Join(tmpDir, "subdir", "file2.txt"), []byte("two"), 0644) - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -488,7 +487,7 @@ func TestUnixTransport_E2E_DirList(t *testing.T) { scanner := bufio.NewScanner(conn) scanner.Buffer(make([]byte, 64*1024), 10*1024*1024) - // Initialize + // Initialise conn.Write([]byte(jsonRPCRequest(1, "initialize", map[string]any{ "protocolVersion": "2024-11-05", "capabilities": map[string]any{}, @@ -572,7 +571,7 @@ func assertToolExists(t *testing.T, tools []any, name string) { func TestTCPTransport_E2E_ToolsDiscovery(t *testing.T) { tmpDir := t.TempDir() - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -610,7 +609,7 @@ func TestTCPTransport_E2E_ToolsDiscovery(t *testing.T) { scanner := bufio.NewScanner(conn) scanner.Buffer(make([]byte, 64*1024), 10*1024*1024) - // Initialize + // Initialise conn.Write([]byte(jsonRPCRequest(1, "initialize", map[string]any{ "protocolVersion": "2024-11-05", "capabilities": map[string]any{}, @@ -648,7 +647,7 @@ func TestTCPTransport_E2E_ToolsDiscovery(t *testing.T) { func TestTCPTransport_E2E_ErrorHandling(t *testing.T) { tmpDir := t.TempDir() - s, err := New(WithWorkspaceRoot(tmpDir)) + s, err := New(Options{WorkspaceRoot: tmpDir}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -686,7 +685,7 @@ func TestTCPTransport_E2E_ErrorHandling(t *testing.T) { scanner := bufio.NewScanner(conn) scanner.Buffer(make([]byte, 64*1024), 10*1024*1024) - // Initialize + // Initialise conn.Write([]byte(jsonRPCRequest(1, "initialize", map[string]any{ "protocolVersion": "2024-11-05", "capabilities": map[string]any{}, @@ -737,6 +736,3 @@ func TestTCPTransport_E2E_ErrorHandling(t *testing.T) { cancel() <-errCh } - -// Suppress "unused import" for fmt — used in helpers -var _ = fmt.Sprintf diff --git a/pkg/mcp/transport_http.go b/pkg/mcp/transport_http.go index c0aac62..cd25417 100644 --- a/pkg/mcp/transport_http.go +++ b/pkg/mcp/transport_http.go @@ -8,6 +8,7 @@ import ( "net" "net/http" "os" + "strings" "time" coreerr "forge.lthn.ai/core/go-log" @@ -15,16 +16,22 @@ import ( ) // DefaultHTTPAddr is the default address for the MCP HTTP server. +// +// svc.ServeHTTP(ctx, DefaultHTTPAddr) // "127.0.0.1:9101" const DefaultHTTPAddr = "127.0.0.1:9101" // ServeHTTP starts the MCP server with Streamable HTTP transport. // Supports Bearer token authentication via MCP_AUTH_TOKEN env var. // If no token is set, authentication is disabled (local development mode). // -// The server exposes a single endpoint at /mcp that handles: -// - GET: Open SSE stream for server-to-client notifications -// - POST: Send JSON-RPC messages (tool calls, etc.) -// - DELETE: Terminate session +// // Local development (no auth): +// svc.ServeHTTP(ctx, "127.0.0.1:9101") +// +// // Production (with auth): +// os.Setenv("MCP_AUTH_TOKEN", "sk-abc123") +// svc.ServeHTTP(ctx, "0.0.0.0:9101") +// +// Endpoint /mcp: GET (SSE stream), POST (JSON-RPC), DELETE (terminate session). func (s *Service) ServeHTTP(ctx context.Context, addr string) error { if addr == "" { addr = DefaultHTTPAddr @@ -75,18 +82,27 @@ func (s *Service) ServeHTTP(ctx context.Context, addr string) error { } // withAuth wraps an http.Handler with Bearer token authentication. -// If token is empty, authentication is disabled (passthrough). +// If token is empty, requests are rejected. func withAuth(token string, next http.Handler) http.Handler { - if token == "" { - return next - } return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.TrimSpace(token) == "" { + w.Header().Set("WWW-Authenticate", `Bearer`) + http.Error(w, `{"error":"authentication not configured"}`, http.StatusUnauthorized) + return + } + auth := r.Header.Get("Authorization") - if len(auth) < 7 || auth[:7] != "Bearer " { + if !strings.HasPrefix(auth, "Bearer ") { http.Error(w, `{"error":"missing Bearer token"}`, http.StatusUnauthorized) return } - provided := auth[7:] + + provided := strings.TrimSpace(strings.TrimPrefix(auth, "Bearer ")) + if len(provided) == 0 { + http.Error(w, `{"error":"missing Bearer token"}`, http.StatusUnauthorized) + return + } + if subtle.ConstantTimeCompare([]byte(provided), []byte(token)) != 1 { http.Error(w, `{"error":"invalid token"}`, http.StatusUnauthorized) return diff --git a/pkg/mcp/transport_http_test.go b/pkg/mcp/transport_http_test.go new file mode 100644 index 0000000..ec8dc10 --- /dev/null +++ b/pkg/mcp/transport_http_test.go @@ -0,0 +1,250 @@ +// SPDX-License-Identifier: EUPL-1.2 + +package mcp + +import ( + "context" + "fmt" + "net" + "net/http" + "os" + "testing" + "time" +) + +func TestServeHTTP_Good_HealthEndpoint(t *testing.T) { + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Get a free port + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("Failed to find free port: %v", err) + } + addr := listener.Addr().String() + listener.Close() + + errCh := make(chan error, 1) + go func() { + errCh <- s.ServeHTTP(ctx, addr) + }() + + // Wait for server to start + time.Sleep(100 * time.Millisecond) + + resp, err := http.Get(fmt.Sprintf("http://%s/health", addr)) + if err != nil { + t.Fatalf("health check failed: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != 200 { + t.Errorf("expected 200, got %d", resp.StatusCode) + } + + cancel() + <-errCh +} + +func TestServeHTTP_Good_DefaultAddr(t *testing.T) { + if DefaultHTTPAddr != "127.0.0.1:9101" { + t.Errorf("expected default HTTP addr 127.0.0.1:9101, got %s", DefaultHTTPAddr) + } +} + +func TestServeHTTP_Good_AuthRequired(t *testing.T) { + os.Setenv("MCP_AUTH_TOKEN", "test-secret-token") + defer os.Unsetenv("MCP_AUTH_TOKEN") + + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("Failed to find free port: %v", err) + } + addr := listener.Addr().String() + listener.Close() + + errCh := make(chan error, 1) + go func() { + errCh <- s.ServeHTTP(ctx, addr) + }() + + time.Sleep(100 * time.Millisecond) + + // Request without token should be rejected + resp, err := http.Get(fmt.Sprintf("http://%s/mcp", addr)) + if err != nil { + t.Fatalf("request failed: %v", err) + } + resp.Body.Close() + if resp.StatusCode != 401 { + t.Errorf("expected 401 without token, got %d", resp.StatusCode) + } + + // Health endpoint should still work (no auth) + resp, err = http.Get(fmt.Sprintf("http://%s/health", addr)) + if err != nil { + t.Fatalf("health check failed: %v", err) + } + resp.Body.Close() + if resp.StatusCode != 200 { + t.Errorf("expected 200 for health, got %d", resp.StatusCode) + } + + cancel() + <-errCh +} + +func TestWithAuth_Good_ValidToken(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }) + + wrapped := withAuth("my-token", handler) + + // Valid token + req, _ := http.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "Bearer my-token") + rr := &fakeResponseWriter{code: 200} + wrapped.ServeHTTP(rr, req) + if rr.code != 200 { + t.Errorf("expected 200 with valid token, got %d", rr.code) + } +} + +func TestWithAuth_Bad_InvalidToken(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }) + + wrapped := withAuth("my-token", handler) + + // Wrong token + req, _ := http.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "Bearer wrong-token") + rr := &fakeResponseWriter{code: 200} + wrapped.ServeHTTP(rr, req) + if rr.code != 401 { + t.Errorf("expected 401 with wrong token, got %d", rr.code) + } +} + +func TestWithAuth_Bad_MissingToken(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }) + + wrapped := withAuth("my-token", handler) + + // No Authorization header + req, _ := http.NewRequest("GET", "/", nil) + rr := &fakeResponseWriter{code: 200} + wrapped.ServeHTTP(rr, req) + if rr.code != 401 { + t.Errorf("expected 401 with missing token, got %d", rr.code) + } +} + +func TestWithAuth_Bad_EmptyConfiguredToken(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }) + + // Empty token now requires explicit configuration + wrapped := withAuth("", handler) + + req, _ := http.NewRequest("GET", "/", nil) + rr := &fakeResponseWriter{code: 200} + wrapped.ServeHTTP(rr, req) + if rr.code != 401 { + t.Errorf("expected 401 with empty configured token, got %d", rr.code) + } +} + +func TestWithAuth_Bad_NonBearerToken(t *testing.T) { + handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }) + + wrapped := withAuth("my-token", handler) + + req, _ := http.NewRequest("GET", "/", nil) + req.Header.Set("Authorization", "Token my-token") + rr := &fakeResponseWriter{code: 200} + wrapped.ServeHTTP(rr, req) + if rr.code != 401 { + t.Errorf("expected 401 with non-Bearer auth, got %d", rr.code) + } +} + +func TestRun_Good_HTTPTrigger(t *testing.T) { + s, err := New(Options{}) + if err != nil { + t.Fatalf("Failed to create service: %v", err) + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + // Find a free port + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("Failed to find free port: %v", err) + } + addr := listener.Addr().String() + listener.Close() + + // MCP_HTTP_ADDR takes priority over MCP_ADDR + os.Setenv("MCP_HTTP_ADDR", addr) + os.Setenv("MCP_ADDR", "") + defer os.Unsetenv("MCP_HTTP_ADDR") + defer os.Unsetenv("MCP_ADDR") + + errCh := make(chan error, 1) + go func() { + errCh <- s.Run(ctx) + }() + + time.Sleep(100 * time.Millisecond) + + // Verify server is running + resp, err := http.Get(fmt.Sprintf("http://%s/health", addr)) + if err != nil { + t.Fatalf("health check failed: %v", err) + } + resp.Body.Close() + if resp.StatusCode != 200 { + t.Errorf("expected 200, got %d", resp.StatusCode) + } + + cancel() + <-errCh +} + +// fakeResponseWriter is a minimal http.ResponseWriter for unit testing withAuth. +type fakeResponseWriter struct { + code int + hdr http.Header +} + +func (f *fakeResponseWriter) Header() http.Header { + if f.hdr == nil { + f.hdr = make(http.Header) + } + return f.hdr +} + +func (f *fakeResponseWriter) Write(b []byte) (int, error) { return len(b), nil } +func (f *fakeResponseWriter) WriteHeader(code int) { f.code = code } diff --git a/pkg/mcp/transport_stdio.go b/pkg/mcp/transport_stdio.go index 10ea27c..2cc39ca 100644 --- a/pkg/mcp/transport_stdio.go +++ b/pkg/mcp/transport_stdio.go @@ -9,6 +9,10 @@ import ( // ServeStdio starts the MCP server over stdin/stdout. // This is the default transport for CLI integrations. +// +// if err := svc.ServeStdio(ctx); err != nil { +// log.Fatal("stdio transport failed", "err", err) +// } func (s *Service) ServeStdio(ctx context.Context) error { s.logger.Info("MCP Stdio server starting", "user", log.Username()) return s.server.Run(ctx, &mcp.StdioTransport{}) diff --git a/pkg/mcp/transport_tcp.go b/pkg/mcp/transport_tcp.go index eb7ec91..860bd18 100644 --- a/pkg/mcp/transport_tcp.go +++ b/pkg/mcp/transport_tcp.go @@ -4,7 +4,7 @@ import ( "bufio" "context" "fmt" - "io" + goio "io" "net" "os" "sync" @@ -14,6 +14,8 @@ import ( ) // DefaultTCPAddr is the default address for the MCP TCP server. +// +// t, err := NewTCPTransport(DefaultTCPAddr) // "127.0.0.1:9100" const DefaultTCPAddr = "127.0.0.1:9100" // diagMu protects diagWriter from concurrent access across tests and goroutines. @@ -21,7 +23,7 @@ var diagMu sync.Mutex // diagWriter is the destination for warning and diagnostic messages. // Use diagPrintf to write to it safely. -var diagWriter io.Writer = os.Stderr +var diagWriter goio.Writer = os.Stderr // diagPrintf writes a formatted message to diagWriter under the mutex. func diagPrintf(format string, args ...any) { @@ -32,7 +34,7 @@ func diagPrintf(format string, args ...any) { // setDiagWriter swaps the diagnostic writer and returns the previous one. // Used by tests to capture output without racing. -func setDiagWriter(w io.Writer) io.Writer { +func setDiagWriter(w goio.Writer) goio.Writer { diagMu.Lock() defer diagMu.Unlock() old := diagWriter @@ -44,15 +46,19 @@ func setDiagWriter(w io.Writer) io.Writer { const maxMCPMessageSize = 10 * 1024 * 1024 // TCPTransport manages a TCP listener for MCP. +// +// t, err := NewTCPTransport("127.0.0.1:9100") type TCPTransport struct { addr string listener net.Listener } // NewTCPTransport creates a new TCP transport listener. -// It listens on the provided address (e.g. "localhost:9100"). // Defaults to 127.0.0.1 when the host component is empty (e.g. ":9100"). // Emits a security warning when explicitly binding to 0.0.0.0 (all interfaces). +// +// t, err := NewTCPTransport("127.0.0.1:9100") +// t, err := NewTCPTransport(":9100") // defaults to 127.0.0.1:9100 func NewTCPTransport(addr string) (*TCPTransport, error) { host, port, _ := net.SplitHostPort(addr) if host == "" { @@ -69,6 +75,10 @@ func NewTCPTransport(addr string) (*TCPTransport, error) { // ServeTCP starts a TCP server for the MCP service. // It accepts connections and spawns a new MCP server session for each connection. +// +// if err := svc.ServeTCP(ctx, "127.0.0.1:9100"); err != nil { +// log.Fatal("tcp transport failed", "err", err) +// } func (s *Service) ServeTCP(ctx context.Context, addr string) error { t, err := NewTCPTransport(addr) if err != nil { @@ -104,23 +114,18 @@ func (s *Service) ServeTCP(ctx context.Context, addr string) error { } func (s *Service) handleConnection(ctx context.Context, conn net.Conn) { - // Note: We don't defer conn.Close() here because it's closed by the Server/Transport - - // Create new server instance for this connection - impl := &mcp.Implementation{ - Name: "core-cli", - Version: "0.1.0", - } - server := mcp.NewServer(impl, nil) - s.registerTools(server) - - // Create transport for this connection + // Connect this TCP connection to the shared server so its session + // is visible to Sessions() and notification broadcasting. transport := &connTransport{conn: conn} - - // Run server (blocks until connection closed) - // Server.Run calls Connect, then Read loop. - if err := server.Run(ctx, transport); err != nil { + session, err := s.server.Connect(ctx, transport, nil) + if err != nil { diagPrintf("Connection error: %v\n", err) + conn.Close() + return + } + // Block until the session ends + if err := session.Wait(); err != nil { + diagPrintf("Session ended: %v\n", err) } } @@ -151,7 +156,7 @@ func (c *connConnection) Read(ctx context.Context) (jsonrpc.Message, error) { return nil, err } // EOF - connection closed cleanly - return nil, io.EOF + return nil, goio.EOF } line := c.scanner.Bytes() return jsonrpc.DecodeMessage(line) @@ -173,5 +178,5 @@ func (c *connConnection) Close() error { } func (c *connConnection) SessionID() string { - return "tcp-session" // Unique ID might be better, but optional + return "tcp-" + c.conn.RemoteAddr().String() } diff --git a/pkg/mcp/transport_tcp_test.go b/pkg/mcp/transport_tcp_test.go index ba9a229..109a617 100644 --- a/pkg/mcp/transport_tcp_test.go +++ b/pkg/mcp/transport_tcp_test.go @@ -37,8 +37,8 @@ func TestNewTCPTransport_Warning(t *testing.T) { old := setDiagWriter(&buf) defer setDiagWriter(old) - // Trigger warning - tr, err := NewTCPTransport("0.0.0.0:9101") + // Trigger warning — use port 0 (OS assigns free port) + tr, err := NewTCPTransport("0.0.0.0:0") if err != nil { t.Fatalf("Failed to create transport: %v", err) } @@ -51,7 +51,7 @@ func TestNewTCPTransport_Warning(t *testing.T) { } func TestServeTCP_Connection(t *testing.T) { - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -101,7 +101,7 @@ func TestServeTCP_Connection(t *testing.T) { } func TestRun_TCPTrigger(t *testing.T) { - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } @@ -139,7 +139,7 @@ func TestRun_TCPTrigger(t *testing.T) { } func TestServeTCP_MultipleConnections(t *testing.T) { - s, err := New() + s, err := New(Options{}) if err != nil { t.Fatalf("Failed to create service: %v", err) } diff --git a/pkg/mcp/transport_unix.go b/pkg/mcp/transport_unix.go index 0506231..ab0e459 100644 --- a/pkg/mcp/transport_unix.go +++ b/pkg/mcp/transport_unix.go @@ -10,7 +10,10 @@ import ( // ServeUnix starts a Unix domain socket server for the MCP service. // The socket file is created at the given path and removed on shutdown. -// It accepts connections and spawns a new MCP server session for each connection. +// +// if err := svc.ServeUnix(ctx, "/tmp/core-mcp.sock"); err != nil { +// log.Fatal("unix transport failed", "err", err) +// } func (s *Service) ServeUnix(ctx context.Context, socketPath string) error { // Clean up any stale socket file if err := io.Local.Delete(socketPath); err != nil { diff --git a/src/php/src/Front/View/Blade/layouts/mcp.blade.php b/src/php/src/Front/View/Blade/layouts/mcp.blade.php index 04f0e9a..ea4155a 100644 --- a/src/php/src/Front/View/Blade/layouts/mcp.blade.php +++ b/src/php/src/Front/View/Blade/layouts/mcp.blade.php @@ -50,7 +50,7 @@ {{ $workspace->name }} @else - + Sign in @endif diff --git a/src/php/src/Mcp/Middleware/McpAuthenticate.php b/src/php/src/Mcp/Middleware/McpAuthenticate.php index 28abaae..1fb161f 100644 --- a/src/php/src/Mcp/Middleware/McpAuthenticate.php +++ b/src/php/src/Mcp/Middleware/McpAuthenticate.php @@ -113,6 +113,6 @@ class McpAuthenticate ], 401); } - return redirect()->guest(route('login')); + return redirect()->guest(url('/login')); } } diff --git a/src/php/src/Mcp/View/Blade/admin/playground.blade.php b/src/php/src/Mcp/View/Blade/admin/playground.blade.php index 1077ee5..5b5b708 100644 --- a/src/php/src/Mcp/View/Blade/admin/playground.blade.php +++ b/src/php/src/Mcp/View/Blade/admin/playground.blade.php @@ -85,7 +85,7 @@ @elseif(!$isAuthenticated && !$apiKey)
- {{ __('mcp::mcp.playground.auth.sign_in_prompt') }} + {{ __('mcp::mcp.playground.auth.sign_in_prompt') }} {{ __('mcp::mcp.playground.auth.sign_in_description') }}