Skip to content

feat(core): define handler contract and extract statusHandler#164

Open
galligan wants to merge 1 commit into
fire-3/contracts-upgradefrom
fire-4/handler-contract
Open

feat(core): define handler contract and extract statusHandler#164
galligan wants to merge 1 commit into
fire-3/contracts-upgradefrom
fire-4/handler-contract

Conversation

@galligan

@galligan galligan commented Feb 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Define HandlerContext and Handler<TInput, TOutput, TError> in packages/core/src/handlers/types.ts
  • Extract statusHandler as first transport-agnostic handler with Result<StatusOutput, Error> return
  • Wire CLI status command and MCP fw_status tool to shared handler
  • Add tests for getCacheStats (empty, populated, multi-repo scenarios) (FIRE-4)

Test plan

  • bun run check passes
  • bun test passes (288 tests)
  • bun apps/cli/bin/fw.ts status --short returns valid JSON

🤘🏻 In-collaboration-with: Claude Code

Greptile Summary

Extracts status logic into a transport-agnostic statusHandler that both CLI and MCP can consume, eliminating ~200 lines of duplication.

  • Defines HandlerContext (config, db, logger) and Handler<TInput, TOutput, TError> contract in packages/core/src/handlers/types.ts
  • Implements statusHandler that gathers auth, config, repo, Graphite, and cache info and returns Result<StatusOutput, Error>
  • Extracts getCacheStats helper for querying repos, entries, and last sync time from the database
  • Refactors CLI status command to delegate to statusHandler, removing all duplicate logic
  • Refactors MCP fw_status tool to use statusHandler, removing duplicate getCacheStats implementation
  • Adds comprehensive tests covering empty, populated, and multi-repo scenarios
  • Adds @outfitter/firewatch-shared dependency to CLI and MCP packages for silentLogger import

Confidence Score: 5/5

  • This PR is safe to merge with no issues found
  • Clean refactoring with comprehensive test coverage (288 tests pass), eliminates code duplication, follows established patterns, and all test plan items verified
  • No files require special attention

Important Files Changed

Filename Overview
packages/core/src/handlers/types.ts Defines HandlerContext and Handler<TInput, TOutput, TError> types for transport-agnostic handlers
packages/core/src/handlers/status.ts Implements statusHandler with getCacheStats helper, returns structured StatusOutput
packages/core/tests/handlers/status.test.ts Comprehensive tests for getCacheStats and statusHandler covering empty, populated, and multi-repo scenarios
apps/cli/src/commands/status.ts Refactored to use statusHandler, removed 150+ lines of duplicate logic
apps/mcp/src/index.ts Refactored fw_status tool to use shared statusHandler, removed duplicate getCacheStats

Flowchart

%%{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&lt;StatusOutput, Error&gt;]
    L -->|CLI formats| M[Console output or JSON]
    L -->|MCP formats| N[MCP tool response]
Loading

Last reviewed commit: f07a741

@linear

linear Bot commented Feb 22, 2026

Copy link
Copy Markdown

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)
@galligan galligan force-pushed the fire-4/handler-contract branch from dbcaf82 to f07a741 Compare February 22, 2026 03:51
@galligan galligan marked this pull request as ready for review February 22, 2026 04:07

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +160 to +163
const dbFile = Bun.file(PATHS.db);
const size = dbFile.size;
if (size > 0) {
cache.size_bytes = size;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant