From 477682d8ab59f63c9f763a557a1760563cdc7056 Mon Sep 17 00:00:00 2001 From: Matt Galligan Date: Sat, 21 Feb 2026 22:47:54 -0500 Subject: [PATCH] docs: update architecture docs for Outfitter stack adoption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update AGENTS.md with handlers layer, @outfitter/cli and @outfitter/mcp references. Rewrite docs/mcp.md for 6-tool architecture with defineTool pattern. Update outfitter-patterns.md to reflect completed adoption phases. 🤘🏻 In-collaboration-with: [Claude Code](https://claude.com/claude-code) --- AGENTS.md | 55 ++- docs/architecture/outfitter-patterns.md | 609 ++++++++++++++++++++++++ docs/mcp.md | 178 ++++--- 3 files changed, 735 insertions(+), 107 deletions(-) create mode 100644 docs/architecture/outfitter-patterns.md diff --git a/AGENTS.md b/AGENTS.md index 036658a..0840336 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -39,13 +39,27 @@ For a local `fw-dev` alias during development: ``` apps/ -├── cli/ # @outfitter/firewatch-cli - Commander-based CLI +├── cli/ # @outfitter/firewatch-cli - CLI (Commander + @outfitter/cli) │ ├── bin/fw.ts # Entry point -│ └── src/commands/ # Command implementations -└── mcp/ # @outfitter/firewatch-mcp - MCP server interface +│ └── src/commands/ # Thin command adapters calling core handlers +└── mcp/ # @outfitter/firewatch-mcp - MCP server (@outfitter/mcp) packages/ ├── core/ # @outfitter/firewatch-core - Pure library logic +│ ├── handlers/ # Transport-agnostic handlers (Result-based) +│ │ ├── types.ts # HandlerContext, Handler +│ │ ├── approve.ts # PR approval +│ │ ├── reject.ts # PR request-changes +│ │ ├── comment.ts # PR-level comments +│ │ ├── reply.ts # Review thread replies +│ │ ├── close.ts # Thread resolution / PR close +│ │ ├── ack.ts # Local acknowledgement +│ │ ├── edit.ts # PR/comment editing +│ │ ├── query.ts # Entry filtering pipeline +│ │ ├── sync.ts # Incremental sync orchestration +│ │ ├── status.ts # Cache/auth/config diagnostics +│ │ ├── doctor.ts # Setup issue diagnosis +│ │ └── index.ts # Barrel exports │ ├── ack.ts # Local acknowledgement tracking │ ├── auth.ts # Adaptive auth: gh CLI → env → config │ ├── authors.ts # Author filtering and bot detection @@ -103,7 +117,18 @@ packages/ ### Key Patterns -**Layered architecture**: Core library functions in `packages/core/` are interface-agnostic. CLI (`apps/cli/`) and MCP server (`apps/mcp/`) wire these to different interfaces. +**Handler contract** (`packages/core/src/handlers/`): All business logic lives in transport-agnostic handlers that take typed input + `HandlerContext` and return `Result`. CLI commands and MCP tools are thin adapters. See `handlers/types.ts` for the contract: + +```typescript +type Handler = ( + input: TInput, + ctx: HandlerContext +) => Promise>; +``` + +**Layered architecture**: Core library functions in `packages/core/` are interface-agnostic. CLI (`apps/cli/`) uses `@outfitter/cli` (`createCLI`, `exitWithError`). MCP server (`apps/mcp/`) uses `@outfitter/mcp` (`createMcpServer`, `defineTool`, `connectStdio`). Both call the same core handlers. + +**Result types** (`@outfitter/contracts`): All core functions return `Result` using taxonomy errors (`AuthError`, `NetworkError`, `NotFoundError`, `ValidationError`). CLI maps errors to exit codes via `exitWithError()`. MCP maps errors to JSON-RPC error codes automatically. **Adaptive auth** (`packages/core/src/auth.ts`): Tries gh CLI first, then environment variables, then config file token. @@ -143,23 +168,23 @@ XDG-compliant paths (cross-platform via `env-paths`): ### MCP Server -The MCP server (`apps/mcp/`) exposes 6 tools with auth-gated write operations. +The MCP server (`apps/mcp/`) uses `@outfitter/mcp` with `createMcpServer()`, `defineTool()`, and `connectStdio()`. It exposes 6 tools with auth-gated write operations and MCP tool annotations. **Base tools (always available):** -| Tool | Description | -| ----------- | ------------------------------------------------------ | -| `fw_query` | Query cached PR activity (supports summary, filters) | -| `fw_status` | Cache and auth status (`short` for compact) | -| `fw_doctor` | Diagnose auth/cache/repo issues (`fix` to auto-repair) | -| `fw_help` | Usage docs, JSON schemas, config inspection | +| Tool | Annotations | Description | +| ----------- | --------------- | ------------------------------------------------------ | +| `fw_query` | readOnly | Query cached PR activity (supports summary, filters) | +| `fw_status` | readOnly | Cache and auth status (`short` for compact) | +| `fw_doctor` | readOnly | Diagnose auth/cache/repo issues (`fix` to auto-repair) | +| `fw_help` | readOnly | Usage docs, JSON schemas, config inspection | **Write tools (require authentication):** -| Tool | Description | -| ------- | ---------------------------------------------------------- | -| `fw_pr` | PR mutations: edit fields, manage metadata, submit reviews | -| `fw_fb` | Unified feedback: list, view, reply, ack, resolve | +| Tool | Annotations | Description | +| ------- | --------------- | ---------------------------------------------------------- | +| `fw_pr` | destructive | PR mutations: edit fields, manage metadata, submit reviews | +| `fw_fb` | destructive | Unified feedback: list, view, reply, ack, resolve | **Example calls:** diff --git a/docs/architecture/outfitter-patterns.md b/docs/architecture/outfitter-patterns.md new file mode 100644 index 0000000..284a0ff --- /dev/null +++ b/docs/architecture/outfitter-patterns.md @@ -0,0 +1,609 @@ +# Outfitter Stack Patterns Guide for Firewatch + +Reference for bringing Firewatch in line with the full Outfitter stack. Covers what packages are available, what Firewatch already uses, what patterns apply, and the concrete steps to adopt them. + +## Package Landscape + +All packages published to npm under `@outfitter/*`. Versions as of 2026-02-21. + +### Currently Used + +| Package | Firewatch Version | Latest | Used In | +| ---------------------- | ----------------- | ------ | ---------------- | +| `@outfitter/contracts` | ^0.4.1 | 0.4.1 | core, shared | +| `@outfitter/cli` | ^0.5.2 | 0.5.2 | cli | +| `@outfitter/mcp` | ^0.4.2 | 0.4.2 | mcp | +| `@outfitter/logging` | ^0.4.1 | 0.4.1 | mcp (transitive) | +| `@outfitter/config` | ^0.3.3 | 0.3.3 | cli (transitive) | +| `@outfitter/schema` | ^0.2.2 | 0.2.2 | cli (transitive) | +| `@outfitter/types` | ^0.2.3 | 0.2.3 | cli (transitive) | + +### Available for Adoption + +| Package | Latest | What It Provides | Priority | +| --------------------- | ------ | ------------------------------------------------------------------ | -------- | +| `@outfitter/logging` | 0.4.1 | Structured logging, redaction, sinks, child loggers (direct usage) | Medium | +| `@outfitter/config` | 0.3.3 | XDG-compliant paths, config loading (direct usage) | Medium | +| `@outfitter/tui` | 0.2.1 | Rendering (tables, trees, boxes), spinners, prompts | Medium | +| `@outfitter/testing` | 0.2.4 | Test harnesses, fixtures, `withTempDir`, `withEnv` | Medium | +| `@outfitter/tooling` | 0.3.0 | Biome/TypeScript/Lefthook presets (dev dep) | Low | +| `@outfitter/state` | 0.2.3 | Pagination cursors, state management | Low | +| `@outfitter/index` | 0.2.3 | SQLite FTS5, WAL mode, BM25 ranking | Low | +| `@outfitter/file-ops` | 0.2.3 | Atomic writes, file locking, secure paths | Low | +| `@outfitter/daemon` | 0.2.4 | Background service lifecycle, IPC, health checks | Low | + +--- + +## Pattern 1: Handler Contract (Transport-Agnostic Logic) + +The central pattern in the Outfitter stack. Handlers are pure functions that accept typed input and context, returning `Result`. CLI and MCP are thin adapters. + +### Current State in Firewatch + +Firewatch has a **complete** handler layer in `packages/core/src/handlers/`. All 12 operations (approve, reject, comment, reply, close, ack, edit, query, sync, status, doctor) are implemented as transport-agnostic handlers returning `Result`. The CLI commands call handlers via thin adapters using `exitWithError()` from `@outfitter/cli`. The MCP tools use `defineTool()` from `@outfitter/mcp` and call the same handlers. + +### Target Pattern + +```typescript +// packages/core/src/handlers/approve.ts +import { Result, type Handler } from "@outfitter/contracts"; +import { z } from "zod"; + +const ApproveInputSchema = z.object({ + pr: z.number().int().positive(), + repo: z.string().min(1), + body: z.string().optional(), +}); + +type ApproveInput = z.infer; + +interface ApproveOutput { + repo: string; + pr: number; + reviewId?: string; + url?: string; +} + +type ApproveErrors = AuthError | NotFoundError | NetworkError; + +export const approveHandler: Handler< + ApproveInput, + ApproveOutput, + ApproveErrors +> = async (input, ctx) => { + ctx.logger.debug("Approving PR", { repo: input.repo, pr: input.pr }); + + const [owner, name] = input.repo.split("/"); + const client = ctx.config?.get("githubClient"); + + const reviewResult = await client.addReview( + owner, + name, + input.pr, + "approve", + input.body + ); + if (reviewResult.isErr()) { + return reviewResult; + } + + const review = reviewResult.value; + return Result.ok({ + repo: input.repo, + pr: input.pr, + ...(review?.id && { reviewId: review.id }), + ...(review?.url && { url: review.url }), + }); +}; +``` + +### CLI Adapter (Thin Wrapper) + +```typescript +// apps/cli/src/commands/approve.ts +import { exitWithError } from "@outfitter/cli/output"; +import { output } from "@outfitter/cli"; +import { approveHandler } from "@outfitter/firewatch-core/handlers"; + +export const approveCommand = command("approve") + .argument("", "PR number", parseInt) + .option("--repo ", "Repository (owner/repo)") + .option("-b, --body ", "Approval message") + .action(async ({ args, flags }) => { + const result = await approveHandler( + { pr: args.pr, repo: flags.repo, body: flags.body }, + ctx + ); + + if (result.isErr()) { + exitWithError(result.error); + } + + await output(result.value, { mode: flags.json ? "json" : undefined }); + }) + .build(); +``` + +### MCP Adapter (Thin Wrapper) + +```typescript +// apps/mcp/src/tools/approve.ts +import { defineTool } from "@outfitter/mcp"; +import { approveHandler } from "@outfitter/firewatch-core/handlers"; + +export const approveTool = defineTool({ + name: "fw_approve", + description: "Approve a PR", + inputSchema: ApproveInputSchema, + annotations: { + readOnlyHint: false, + destructiveHint: false, + idempotentHint: true, + }, + handler: async (input, ctx) => approveHandler(input, ctx), +}); +``` + +### Handler Coverage + +All operations have core handlers in `packages/core/src/handlers/`. CLI commands and MCP tools are thin adapters. + +| Operation | Core Handler | CLI Adapter | MCP Tool | +| --------- | ------------------ | ------------- | ----------- | +| approve | `approveHandler` | `approve.ts` | `fw_pr` | +| reject | `rejectHandler` | `reject.ts` | `fw_pr` | +| comment | `commentHandler` | `comment.ts` | `fw_fb` | +| reply | `replyHandler` | `reply.ts` | `fw_fb` | +| close | `closeHandler` | `close.ts` | `fw_fb` | +| edit | `editHandler` | `edit.ts` | `fw_pr` | +| ack | `ackHandler` | `ack.ts` | `fw_fb` | +| query | `queryHandler` | `query.ts` | `fw_query` | +| sync | `syncHandler` | `sync.ts` | (internal) | +| status | `statusHandler` | `status.ts` | `fw_status` | +| doctor | `doctorHandler` | `doctor.ts` | `fw_doctor` | +| freeze | `freezePR` | `freeze.ts` | N/A | +| unfreeze | `unfreezePR` | `unfreeze.ts` | N/A | + +--- + +## Pattern 2: Error Taxonomy + +### Current State + +Firewatch uses `AuthError`, `NetworkError`, and `NotFoundError` from contracts across core, CLI, and MCP. Core handlers return `Result.err()` with taxonomy errors. The CLI uses `exitWithError()` from `@outfitter/cli` which maps error categories to exit codes. The MCP server uses `adaptHandler()` from `@outfitter/mcp` to translate errors to JSON-RPC codes. Some CLI commands still have direct `console.error` + `process.exit(1)` for edge cases not covered by handlers (e.g., list, view, config). + +### Target: Map All Failures to Taxonomy + +| Current Pattern | Maps To | Category | Exit Code | +| ------------------------------------------------- | ----------------- | ---------- | --------- | +| `throw new Error("requires pr")` | `ValidationError` | validation | 1 | +| `throw new Error("not found")` | `NotFoundError` | not_found | 2 | +| `throw new Error("auth required")` | `AuthError` | auth | 9 | +| `throw new Error("invalid format")` | `ValidationError` | validation | 1 | +| `throw new Error("sync failed")` | `NetworkError` | network | 7 | +| `throw new Error("config updates not supported")` | `PermissionError` | permission | 4 | + +### CLI Error Handling + +Replace: + +```typescript +// Before +try { + await doSomething(); +} catch (error) { + console.error("Failed:", error instanceof Error ? error.message : error); + process.exit(1); +} +``` + +With: + +```typescript +// After (using @outfitter/cli) +import { exitWithError } from "@outfitter/cli/output"; + +const result = await handler(input, ctx); +if (result.isErr()) { + exitWithError(result.error); // Exit code from error.category +} +``` + +### MCP Error Handling + +Replace: + +```typescript +// Before +if (auth.isErr()) { + throw new Error(auth.error.message); // Loses error category +} +``` + +With: + +```typescript +// After (handler returns Result, @outfitter/mcp translates automatically) +if (auth.isErr()) { + return auth; // Error category maps to JSON-RPC code +} +``` + +--- + +## Pattern 3: Structured Logging + +### Current State + +Firewatch has a custom logger in `packages/shared/src/logger.ts` that satisfies the `Logger` interface from `@outfitter/contracts`. It supports levels, child loggers, and stderr routing. But it lacks redaction, sinks, and environment-aware level resolution. + +The CLI commands use `console.error()` and `console.log()` directly (~104 and ~60 instances respectively). + +### Target: `@outfitter/logging` + +```typescript +// Replace packages/shared/src/logger.ts with: +import { + createLogger, + createConsoleSink, + resolveLogLevel, +} from "@outfitter/logging"; + +export function createFirewatchLogger(options?: { silent?: boolean }) { + return createLogger({ + name: "firewatch", + level: options?.silent ? "silent" : resolveLogLevel(), + sinks: [createConsoleSink()], + redaction: { enabled: true }, // Auto-redacts tokens, API keys + }); +} + +export const silentLogger = createLogger({ + name: "firewatch", + level: "silent", + sinks: [], +}); +``` + +Benefits: + +- **Redaction**: GitHub tokens (`ghp_*`, `gho_*`) automatically redacted in log output +- **Environment-aware levels**: `OUTFITTER_LOG_LEVEL=debug` or `OUTFITTER_ENV=development` controls verbosity +- **Child loggers with context**: `logger.child({ repo: "outfitter-dev/firewatch", command: "sync" })` +- **File sink**: Could log to `~/.local/state/firewatch/debug.log` for diagnostics + +### Migration Path + +1. Install `@outfitter/logging` +2. Replace `createLogger` in shared with the `@outfitter/logging` version +3. Keep the `Logger` interface from `@outfitter/contracts` (both implementations satisfy it) +4. Gradually replace `console.error`/`console.log` calls with `ctx.logger.*` calls + +--- + +## Pattern 4: CLI Framework (`@outfitter/cli`) + +### Current State + +Firewatch uses `createCLI()` from `@outfitter/cli` as the CLI entry point, providing global `--json` flag handling and `exitWithError()` for category-mapped exit codes. Commands are registered via `cli.register()` and use Commander v14. Key commands (approve, reject, comment, status, doctor, reply, edit) use `exitWithError()` for error handling. + +### Target: `@outfitter/cli` + +```typescript +// apps/cli/bin/fw.ts +import { createCLI } from "@outfitter/cli/command"; +import { syncCommand } from "./commands/sync.js"; +import { queryCommand } from "./commands/query.js"; + +const cli = createCLI({ + name: "fw", + version: VERSION, + description: "GitHub PR activity logger with JSONL output for jq composition", +}); + +cli.register(syncCommand); +cli.register(queryCommand); +// ... + +await cli.parse(); +``` + +### Key Benefits + +| Feature | Current (raw Commander) | With `@outfitter/cli` | +| ---------------- | ------------------------------------------- | ------------------------------ | +| `--json` flag | Manual per-command (`--jsonl`) | Global, automatic | +| Error exit codes | Always `1` | Category-mapped (1-9, 130) | +| Output mode | `shouldOutputJson()` + `outputStructured()` | `output()` with auto-detection | +| Verbose mode | `--debug` flag, manual | `resolveVerbose()`, env-aware | +| Spinner | `ora` directly | `@outfitter/tui/streaming` | + +### Rendering with `@outfitter/tui` + +Firewatch already has custom rendering in `packages/core/src/render/` (box chars, tree rendering, truncation). These could be replaced with or complemented by `@outfitter/tui/render`: + +```typescript +import { + renderTable, + renderTree, + formatDuration, + pluralize, +} from "@outfitter/tui/render"; +``` + +--- + +## Pattern 5: MCP Server (`@outfitter/mcp`) + +### Current State + +Firewatch uses `createMcpServer()` and `defineTool()` from `@outfitter/mcp` with `connectStdio()` for transport. All 6 tools use `defineTool()` with `TOOL_ANNOTATIONS` presets (readOnly, destructive). Auth-gated write tools are registered after `connectStdio()` with `notifyToolsChanged()`. The `@modelcontextprotocol/sdk` is retained as a devDependency for types. + +### Target: `@outfitter/mcp` + +```typescript +// apps/mcp/src/server.ts +import { createMcpServer } from "@outfitter/mcp"; +import { queryTool } from "./tools/query.js"; +import { statusTool } from "./tools/status.js"; +import { prTool } from "./tools/pr.js"; +import { feedbackTool } from "./tools/feedback.js"; + +const server = createMcpServer({ + name: "firewatch", + version: mcpVersion, + logger: firewatchLogger, +}); + +// Base tools (always available) +server.registerTool(queryTool); +server.registerTool(statusTool); +server.registerTool(doctorTool); +server.registerTool(helpTool); + +// Write tools (auth-gated registration stays manual) +if (authVerified) { + server.registerTool(prTool); + server.registerTool(feedbackTool); +} + +await server.start(); +``` + +### Tool Definition with `defineTool` + +```typescript +// apps/mcp/src/tools/query.ts +import { defineTool } from "@outfitter/mcp"; +import { queryHandler } from "@outfitter/firewatch-core/handlers"; + +export const queryTool = defineTool({ + name: "fw_query", + description: + "Query cached PR activity (reviews, comments, commits, CI). Outputs JSONL.", + inputSchema: QueryInputSchema, + annotations: { + readOnlyHint: true, + destructiveHint: false, + idempotentHint: true, + openWorldHint: true, // May trigger auto-sync to GitHub + }, + deferLoading: false, // Core tool, always listed + handler: async (input, ctx) => queryHandler(input, ctx), +}); +``` + +### Benefits + +- **Tool annotations**: Clients know `fw_query` is read-only, `fw_pr` is destructive +- **Deferred loading**: Domain tools load on demand, core tools always visible +- **Automatic error translation**: `Result.err(NotFoundError)` becomes JSON-RPC `-32601` +- **Log forwarding**: Server can send structured log messages to MCP clients +- **Resource support**: Could expose cached PR data as addressable resources + +--- + +## Pattern 6: XDG Config Paths (`@outfitter/config`) + +### Current State + +Firewatch uses `env-paths` (npm package) for XDG path resolution in `packages/core/src/cache.ts`: + +```typescript +import envPaths from "env-paths"; +const paths = envPaths("firewatch"); +``` + +### Target: `@outfitter/config` + +```typescript +import { + getConfigDir, + getCacheDir, + getDataDir, + getStateDir, +} from "@outfitter/config"; + +const configDir = getConfigDir("firewatch"); // ~/.config/firewatch +const cacheDir = getCacheDir("firewatch"); // ~/.cache/firewatch +const stateDir = getStateDir("firewatch"); // ~/.local/state/firewatch +``` + +This eliminates the `env-paths` dependency. The `@outfitter/config` functions handle XDG env vars (`$XDG_CONFIG_HOME`, etc.) and platform differences. + +--- + +## Pattern 7: Validation with `createValidator` + +### Current State + +Firewatch uses Zod schemas in `packages/core/src/schema/` for type definitions but validates manually in command handlers. The MCP server has separate `*ParamsShape` schemas in `apps/mcp/src/schemas.ts`. + +### Target + +```typescript +import { createValidator } from "@outfitter/contracts"; + +const validateQueryInput = createValidator(QueryInputSchema); + +export const queryHandler: Handler = async ( + rawInput, + ctx +) => { + const inputResult = validateQueryInput(rawInput); + if (inputResult.isErr()) { + return inputResult; // ValidationError with Zod details + } + const input = inputResult.value; // Fully typed + + // Business logic with validated input... +}; +``` + +This replaces the manual validation scattered across CLI and MCP with a single validation point at handler entry. + +--- + +## Pattern 8: Testing + +### Current State + +Firewatch has ~200 tests using `bun:test`. Tests are colocated in `tests/` directories within packages. Core function tests call functions directly. + +### Target Additions with `@outfitter/testing` + +```typescript +import { + createFixture, + withTempDir, + withEnv, + createMockLogger, +} from "@outfitter/testing"; + +// Test fixtures for entries +const createEntry = createFixture({ + id: "test-id", + repo: "owner/repo", + pr: 1, + type: "comment", + author: "testuser", + // ... +}); + +// Environment testing +test("uses custom cache dir", async () => { + await withEnv({ XDG_CACHE_HOME: "/tmp/test-cache" }, async () => { + const paths = getConfigPaths(); + expect(paths.cache).toStartWith("/tmp/test-cache"); + }); +}); + +// Logger verification +test("logs sync progress", async () => { + const mockLogger = createMockLogger(); + const ctx = createContext({ logger: mockLogger }); + + await syncHandler({ repo: "owner/repo" }, ctx); + + expect(mockLogger.calls.info).toContainEqual([ + "Sync complete", + expect.objectContaining({ entriesAdded: expect.any(Number) }), + ]); +}); +``` + +--- + +## Adoption Status + +### Completed + +- **Phase 0**: Upgraded `@outfitter/contracts` to ^0.4.1 +- **Phase 1**: All 12 handlers extracted to `packages/core/src/handlers/` +- **Phase 3**: CLI uses `createCLI()` from `@outfitter/cli`, `exitWithError()` wired on key commands +- **Phase 4**: MCP uses `createMcpServer()` + `defineTool()` from `@outfitter/mcp` + +### Remaining + +- **Phase 2**: Adopt `@outfitter/logging` — Replace custom logger in shared with `@outfitter/logging` for redaction, sinks, environment-aware levels +- **Phase 5**: Adopt `@outfitter/config` — Replace `env-paths` with `getConfigDir()`, `getCacheDir()` from `@outfitter/config` +- **Remaining CLI migration**: Wire list, view, config, freeze, unfreeze, schema, examples commands to use `exitWithError()` and core handlers +- **Remaining output migration**: Replace remaining `console.log`/`console.error` calls with `output()` from `@outfitter/cli` + +--- + +## Remaining Anti-Patterns + +| Anti-Pattern | Location | Fix | +| -------------------------- | ----------------------------------------------- | --------------------------------------------- | +| `process.exit(1)` in CLI | list, view, config, freeze, unfreeze, schema | Use `exitWithError()` | +| `console.error()` in CLI | Same commands as above | Use `ctx.logger.error()` or `exitWithError()` | +| `console.log()` in core | `packages/core/src/query.ts:224` | Return data, let adapter output | +| Custom logger in shared | `packages/shared/src/logger.ts` | Replace with `@outfitter/logging` | +| `env-paths` dep in core | `packages/core/src/cache.ts` | Replace with `@outfitter/config` | + +--- + +## File Layout After Full Adoption + +``` +packages/ + core/ + src/ + handlers/ # NEW: Transport-agnostic handlers + approve.ts + reject.ts + comment.ts + reply.ts + close.ts + ack.ts + edit.ts + freeze.ts + query.ts + sync.ts + status.ts + doctor.ts + index.ts + auth.ts # Already returns Result + github.ts # Already returns Result + time.ts # Already returns Result + freeze.ts # Already returns Result + ... + shared/ + src/ + logger.ts # Replaced by @outfitter/logging + +apps/ + cli/ + src/ + commands/ # Thin adapters calling handlers + approve.ts # ~20 lines: parse args, call handler, output + ... + mcp/ + src/ + tools/ # NEW: defineTool() wrappers + query.ts + status.ts + pr.ts + feedback.ts + index.ts # Simplified: createMcpServer + register tools +``` + +--- + +## Quick Reference: Error Category Mapping + +| Firewatch Domain Error | Taxonomy Error | Category | Exit | HTTP | +| ----------------------- | ----------------- | ---------- | ---- | ---- | +| No auth found | `AuthError` | auth | 9 | 401 | +| PR not found | `NotFoundError` | not_found | 2 | 404 | +| Comment not found | `NotFoundError` | not_found | 2 | 404 | +| Invalid PR number | `ValidationError` | validation | 1 | 400 | +| Invalid duration format | `ValidationError` | validation | 1 | 400 | +| Invalid repo format | `ValidationError` | validation | 1 | 400 | +| GitHub API failure | `NetworkError` | network | 7 | 502 | +| GitHub rate limit | `RateLimitError` | rate_limit | 6 | 429 | +| Config write via MCP | `PermissionError` | permission | 4 | 403 | +| Short ID ambiguous | `ValidationError` | validation | 1 | 400 | +| Sync timeout | `TimeoutError` | timeout | 5 | 504 | +| User cancelled | `CancelledError` | cancelled | 130 | 499 | diff --git a/docs/mcp.md b/docs/mcp.md index 90a6d88..2f0d831 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -1,10 +1,6 @@ # MCP Server - - -Firewatch provides an MCP (Model Context Protocol) server for AI agent integration. The server exposes PR activity data and write operations through a single tool interface. +Firewatch provides an MCP (Model Context Protocol) server for AI agent integration. The server exposes 6 tools with auth-gated write operations, built on `@outfitter/mcp` with `createMcpServer()`, `defineTool()`, and `connectStdio()`. ## Quick Start @@ -46,35 +42,22 @@ Or with the built binary: } ``` -## Tool Design - -Firewatch exposes a **single tool** called `firewatch` with an `action` parameter. +## Tools -### Tool Schema +Firewatch exposes **6 tools** with MCP tool annotations. Base tools are always available; write tools require authentication and are registered dynamically. -```typescript -{ - name: "firewatch", - description: "GitHub PR activity query tool. Outputs JSONL for jq.", - parameters: { - action: "query" | "add" | "close" | "edit" | "rm" | "status" | "config" | "doctor" | "schema" | "help", - // ... additional parameters per action - } -} -``` +### Base Tools (always available) -## Actions +#### `fw_query` (readOnly) -### query - -Filter cached PR activity entries. +Query cached PR activity entries. ```json -{"action": "query", "since": "24h"} -{"action": "query", "type": "review", "author": "alice"} -{"action": "query", "pr": "23,34", "summary": true} -{"action": "query", "states": ["open", "draft"], "limit": 10} -{"action": "query", "summary": true, "summary_short": true} +{"since": "24h"} +{"type": "review", "author": "alice"} +{"pr": "23,34", "summary": true} +{"states": ["open", "draft"], "limit": 10} +{"summary": true, "summary_short": true} ``` **Parameters:** @@ -107,109 +90,120 @@ Filter cached PR activity entries. Note: `mine` and `reviews` require `user.github_username` to be set in config. -### add +#### `fw_status` (readOnly) -Add content or metadata to PRs. +Get Firewatch state information (auth/config/cache). ```json -{"action": "add", "pr": 42, "body": "Thanks for the review!"} -{"action": "add", "pr": 42, "body": "Fixed", "reply_to": "comment-123", "resolve": true} -{"action": "add", "pr": 42, "review": "approve", "body": "LGTM"} -{"action": "add", "pr": 42, "labels": ["bug", "priority-high"]} +{} +{"short": true} ``` -**Parameters:** - -| Parameter | Type | Description | -| ---------- | ------------------ | --------------------------------------- | -| `repo` | string | Repository | -| `pr` | number | PR number (required) | -| `body` | string | Comment/review body | -| `reply_to` | string | Comment ID to reply to | -| `resolve` | boolean | Resolve thread after reply | -| `review` | string | `approve`, `request-changes`, `comment` | -| `labels` | string \| string[] | Labels to add | -| `reviewer` | string \| string[] | Reviewers to request | -| `assignee` | string \| string[] | Assignees to add | - -### close +#### `fw_doctor` (readOnly) -Resolve review comment threads. +Diagnose setup issues (auth, cache, repo, GitHub API). ```json -{"action": "close", "comment_id": "comment-123"} -{"action": "close", "comment_ids": ["comment-123", "comment-456"]} +{} +{"fix": true} ``` -### edit +#### `fw_help` (readOnly) -Update PR fields or state. +Get usage docs, JSON schemas, and config inspection. ```json -{"action": "edit", "pr": 42, "title": "feat: update auth"} -{"action": "edit", "pr": 42, "draft": true} -{"action": "edit", "pr": 42, "milestone": "v1.0"} +{} +{"schema": "entry"} +{"schema": "worklist"} +{"config": true} +{"config_key": "user.github_username"} ``` -### rm +### Write Tools (require authentication) -Remove metadata from PRs. +These tools are registered after auth verification. Clients receive a `tools/list_changed` notification when write tools become available. -```json -{"action": "rm", "pr": 42, "labels": ["wip"]} -{"action": "rm", "pr": 42, "assignee": "galligan"} -{"action": "rm", "pr": 42, "milestone": true} -``` - -### status +#### `fw_pr` (destructive) -Get Firewatch state information (auth/config/cache). +PR mutations: edit fields, manage metadata, submit reviews. ```json -{"action": "status"} -{"action": "status", "status_short": true} +{"pr": 42, "action": "edit", "title": "feat: update auth"} +{"pr": 42, "action": "edit", "draft": true} +{"pr": 42, "action": "add", "review": "approve", "body": "LGTM"} +{"pr": 42, "action": "add", "labels": ["bug", "priority-high"]} +{"pr": 42, "action": "rm", "labels": ["wip"]} ``` -### config +#### `fw_fb` (destructive) -Read Firewatch configuration (read-only). +Unified feedback: list, view, reply, ack, resolve. ```json -{"action": "config"} -{"action": "config", "key": "user.github_username"} -{"action": "config", "path": true} +{"action": "list"} +{"id": "@a7f3c", "action": "view"} +{"id": "@a7f3c", "body": "Fixed", "resolve": true} +{"id": "@a7f3c", "action": "ack"} ``` -### doctor +## Architecture -Diagnose setup (auth, cache, repo, GitHub API). +### Tool Definition Pattern -```json -{ "action": "doctor" } +Tools are defined using `defineTool()` from `@outfitter/mcp` with Zod input schemas and tool annotations: + +```typescript +import { TOOL_ANNOTATIONS, defineTool } from "@outfitter/mcp"; + +const queryTool = defineTool({ + name: "fw_query", + description: "Query cached PR activity", + inputSchema: queryParamsSchema, + annotations: TOOL_ANNOTATIONS.readOnly, + handler: async (params) => { /* ... */ }, +}); ``` -### schema +### Server Creation -Get schema documentation. +The server uses `createMcpServer()` and `connectStdio()` from `@outfitter/mcp`: -```json -{"action": "schema"} -{"action": "schema", "schema": "entry"} -{"action": "schema", "schema": "worklist"} -{"action": "schema", "schema": "config"} +```typescript +const server = createMcpServer({ + name: "firewatch-mcp", + version: mcpVersion, +}); + +// Register base tools +server.registerTool(queryTool); +server.registerTool(statusTool); + +// Start transport +await connectStdio(server); + +// Register write tools after auth check +if (authResult.isOk()) { + server.registerTool(prTool); + server.registerTool(feedbackTool); +} ``` -### help +### Handler Bridge -Get help text. +MCP tool handlers call core handlers via `adaptHandler()` which bridges domain errors to `OutfitterError` for automatic JSON-RPC error code mapping: -```json -{ "action": "help" } +```typescript +handler: adaptHandler(async (params) => { + const result = await statusHandler(input, ctx); + if (result.isErr()) { return result; } + return Result.ok(formatOutput(result.value)); +}), ``` ## Output Format -All actions return JSONL (newline-delimited JSON). Each line is a complete JSON object. +All tools return text content. Query results are JSONL (newline-delimited JSON). ### Query Output @@ -227,7 +221,7 @@ All actions return JSONL (newline-delimited JSON). Each line is a complete JSON ### Operation Results ```json -{ "ok": true, "repo": "org/repo", "pr": 42, "comment_id": "IC_abc123" } +{"ok": true, "repo": "org/repo", "pr": 42, "comment_id": "IC_abc123"} ``` ## Auto-Sync Behavior @@ -242,7 +236,7 @@ The MCP server uses the same authentication chain as the CLI: 2. `GITHUB_TOKEN` / `GH_TOKEN` environment variable 3. `github_token` in config file -Ensure one of these is configured before using the server. +Ensure one of these is configured before using the server. Write tools (`fw_pr`, `fw_fb`) are only registered after successful authentication. ## See Also