refactor: modularize MCP server and CLI shared utilities#162
Conversation
Greptile OverviewGreptile SummaryThis PR modularizes the MCP server implementation by splitting the prior On the MCP side, tool registration is now centralized in On the CLI side, repeated per-command context creation is replaced with a shared One functional regression was found in MCP help/schema output: requesting the Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| apps/cli/src/auth-client.ts | Adds shared tryCreateClient() helper to centralize config/auth/repo resolution and output-format selection. |
| apps/cli/src/repo.ts | Adds resolveRepo() as a silent variant of resolveRepoOrThrow(); keeps repo format validation helpers. |
| apps/mcp/src/handlers/feedback.ts | Extracts feedback tool handler with list/view/reply/ack/resolve flows; uses shared context + ID resolution helpers. |
| apps/mcp/src/handlers/help.ts | Extracts MCP help/schema docs builder; currently returns entry schema for schema:"query" due to missing branch. |
| apps/mcp/src/handlers/mutations.ts | Extracts mutation handlers (add/edit/rm) and shared parsing utilities; preserves prior behavior. |
| apps/mcp/src/handlers/query.ts | Refactors query tool handling into phases (validate, detect, sync, query, filter, format) using extracted utils. |
| apps/mcp/src/index.ts | Replaces monolithic server with FirewatchMCPServer class that registers base tools and auth-gated write tools. |
Sequence Diagram
sequenceDiagram
autonumber
participant Client as MCP Client
participant FW as FirewatchMCPServer
participant Core as firewatch-core
participant DB as SQLite Cache
participant GH as GitHub API
Client->>FW: connect() via stdio transport
FW->>FW: registerBaseTools()
FW->>Core: loadConfig()
FW->>Core: detectAuth(config.github_token)
alt auth token available
FW->>FW: registerWriteTools()
FW-->>Client: sendToolListChanged()
else no auth
FW-->>Client: base tools only
end
Client->>FW: tool call fw_query(params)
FW->>Core: loadConfig()
FW->>Core: detectRepo()
FW->>FW: ensureRepoCacheIfNeeded(repoFilter,...)
alt cache missing or stale + auto_sync
FW->>Core: detectAuth(...)
FW->>GH: syncRepo(...)
GH-->>FW: PR activity
FW->>DB: write entries/meta
end
FW->>DB: queryEntries(filters)
FW-->>Client: JSONL text result
Client->>FW: tool call fw_status({recheck_auth:true})
FW->>Core: loadConfig()
FW->>Core: detectAuth(...)
alt token newly available
FW->>FW: registerWriteTools()
FW-->>Client: sendToolListChanged()
end
FW->>FW: handleStatus(params)
FW-->>Client: status JSON text result
| export function schemaDoc(name: SchemaName | undefined): object { | ||
| if (name === "worklist") { | ||
| return WORKLIST_SCHEMA_DOC; |
There was a problem hiding this comment.
schemaDoc ignores query
schemaDoc() only branches on worklist and config, and otherwise returns ENTRY_SCHEMA_DOC, so fw_help with {"schema":"query"} will return the entry schema instead of the query schema. This is user-visible and breaks the advertised help output for the query schema type.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/mcp/src/handlers/help.ts
Line: 9:11
Comment:
**`schemaDoc` ignores `query`**
`schemaDoc()` only branches on `worklist` and `config`, and otherwise returns `ENTRY_SCHEMA_DOC`, so `fw_help` with `{"schema":"query"}` will return the entry schema instead of the query schema. This is user-visible and breaks the advertised help output for the `query` schema type.
How can I resolve this? If you propose a fix, please make it concise.Split the 2,348-line MCP server monolith into focused modules: - types.ts: shared interfaces (FirewatchParams, McpToolResult) - handlers/: query, status, feedback, mutations, config, help - context/: repo, mutation, feedback context factories - utils/: formatting, parsing, sync, id-resolution - index.ts slimmed to ~290 lines (server class + tool registration) 🤘🏻 In-collaboration-with: [Claude Code](https://claude.com/claude-code)
schemaDoc() silently fell through to ENTRY_SCHEMA_DOC for "query" and "entry" via the default case. Make all SchemaName variants explicit in a switch so the mapping is intentional, not accidental. 🤘🏻 In-collaboration-with: [Claude Code](https://claude.com/claude-code)
0575763 to
08f79bf
Compare

Summary
MCP extraction: Split the 2,348-line
apps/mcp/src/index.tsmonolith into 14 focused modules (~290 lines remaining)types.ts: shared interfaces (FirewatchParams, McpToolResult, SchemaName)handlers/: query, status, feedback, mutations, config, helpcontext/: repo, mutation, feedback context factoriesutils/: formatting, parsing, sync, id-resolutionCLI deduplication: Extract shared auth-client helper and deduplicate resolveRepo
auth-client.tswithtryCreateClient()replacing 6 identicalcreateContextfunctionsresolveRepo()inrepo.tsreplacing 4 inline copies across ack, close, freeze, unfreezeTest plan
bun run check— 0 errors (1 pre-existing complexity warning on handleRm)bun test— 200 tests pass, 518 assertionsfw status,fw list,fw query --type review --since 7d🤘🏻 In-collaboration-with: Claude Code