feat(core): define handler contract and extract statusHandler#164
feat(core): define handler contract and extract statusHandler#164galligan wants to merge 1 commit into
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Introduce the handler pattern — transport-agnostic functions that return Result<T, E> and receive a HandlerContext (config, db, logger). - HandlerContext in packages/core/src/handlers/types.ts - statusHandler extracts shared logic from CLI status command and MCP handleStatus into a single handler in core - CLI status.ts becomes a thin adapter: parse flags → call handler → format - MCP handleStatus calls the same handler, transforms output for MCP shape - 7 new tests for getCacheStats and statusHandler - Mechanical: formatter adjustments from oxfmt run 🤘🏻 In-collaboration-with: [Claude Code](https://claude.com/claude-code)
dbcaf82 to
f07a741
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f07a74114c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const dbFile = Bun.file(PATHS.db); | ||
| const size = dbFile.size; | ||
| if (size > 0) { | ||
| cache.size_bytes = size; |
There was a problem hiding this comment.
Compute cache size from ctx.db instead of global PATHS.db
This handler is intended to be transport-agnostic, but it always reads PATHS.db for size_bytes while counts come from ctx.db. When callers pass an in-memory or non-default database (as tests and future transports can), status output can combine repo/entry counts from one database with file size from another, producing incorrect cache telemetry and violating the handler contract.
Useful? React with 👍 / 👎.

Summary
HandlerContextandHandler<TInput, TOutput, TError>inpackages/core/src/handlers/types.tsstatusHandleras first transport-agnostic handler withResult<StatusOutput, Error>returnstatuscommand and MCPfw_statustool to shared handlergetCacheStats(empty, populated, multi-repo scenarios) (FIRE-4)Test plan
bun run checkpassesbun testpasses (288 tests)bun apps/cli/bin/fw.ts status --shortreturns valid JSON🤘🏻 In-collaboration-with: Claude Code
Greptile Summary
Extracts status logic into a transport-agnostic
statusHandlerthat both CLI and MCP can consume, eliminating ~200 lines of duplication.HandlerContext(config, db, logger) andHandler<TInput, TOutput, TError>contract inpackages/core/src/handlers/types.tsstatusHandlerthat gathers auth, config, repo, Graphite, and cache info and returnsResult<StatusOutput, Error>getCacheStatshelper for querying repos, entries, and last sync time from the databasestatuscommand to delegate tostatusHandler, removing all duplicate logicfw_statustool to usestatusHandler, removing duplicategetCacheStatsimplementation@outfitter/firewatch-shareddependency to CLI and MCP packages forsilentLoggerimportConfidence Score: 5/5
Important Files Changed
HandlerContextandHandler<TInput, TOutput, TError>types for transport-agnostic handlersstatusHandlerwithgetCacheStatshelper, returns structuredStatusOutputgetCacheStatsandstatusHandlercovering empty, populated, and multi-repo scenariosstatusHandler, removed 150+ lines of duplicate logicfw_statustool to use sharedstatusHandler, removed duplicategetCacheStatsFlowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[CLI status command] -->|calls| H[statusHandler] B[MCP fw_status tool] -->|calls| H H -->|receives| C[HandlerContext] C -->|contains| D[config: FirewatchConfig] C -->|contains| E[db: Database] C -->|contains| F[logger: Logger] H -->|calls| G[getCacheStats] G -->|queries| E H -->|calls| I[detectAuth] H -->|calls| J[detectRepo] H -->|calls| K[getGraphiteStacks] H -->|returns| L[Result<StatusOutput, Error>] L -->|CLI formats| M[Console output or JSON] L -->|MCP formats| N[MCP tool response]Last reviewed commit: f07a741