refactor(cli): wire exitWithError() across key commands by galligan · Pull Request #170 · outfitter-dev/firewatch

galligan · 2026-02-22T03:48:33Z

Summary

Wire approveHandler, rejectHandler, commentHandler to core handlers with exitWithError()
Migrate reply, edit, status, doctor commands to use exitWithError() for error paths
Add handler barrel exports to packages/core/src/index.ts
Category-mapped exit codes replace hardcoded process.exit(1) (FIRE-10)

Test plan

bun run check passes
bun test passes (298 tests)

🤘🏻 In-collaboration-with: Claude Code

Greptile Summary

This PR refactors CLI commands to use exitWithError() from @outfitter/cli instead of raw console.error() + process.exit(1) patterns, implementing category-mapped exit codes (FIRE-10).

Key changes:

Migrated approve, reject, and comment commands to use core handlers (approveHandler, rejectHandler, commentHandler) with proper error handling
Updated reply, edit, status, and doctor commands to replace hardcoded process.exit(1) with exitWithError()
Added handler barrel exports to packages/core/src/index.ts for the newly exposed handlers and their types

Issues found:

apps/cli/src/commands/edit.ts has 4 instances where process.exit(1) is called before exitWithError(), making the error handler unreachable. This defeats the purpose of the refactor since exit codes won't be properly categorized in those error paths.

Confidence Score: 3/5

This PR is mostly safe but has unreachable error handlers in edit.ts that need fixing
The refactor successfully migrates most commands to the new error handling pattern, but edit.ts contains critical logic errors where process.exit(1) is called immediately before exitWithError(), making the new error handler unreachable in 4 error paths. This means those error cases won't benefit from category-mapped exit codes, partially defeating the PR's purpose.
apps/cli/src/commands/edit.ts requires attention to fix unreachable error handlers

Important Files Changed

Filename	Overview
apps/cli/src/commands/approve.ts	Migrated to `approveHandler` from core with `exitWithError()` replacing `process.exit(1)`
apps/cli/src/commands/comment.ts	Migrated to `commentHandler` from core with `exitWithError()` replacing `process.exit(1)`
apps/cli/src/commands/reject.ts	Migrated to `rejectHandler` from core with `exitWithError()` replacing `process.exit(1)`
apps/cli/src/commands/edit.ts	Migrated error paths to `exitWithError()`, but has inconsistent error handling with `process.exit(1)` still present before some `exitWithError()` calls
packages/core/src/index.ts	Added barrel exports for new handlers (approve, comment, reject, reply, edit) and their types

_{Last reviewed commit: 056e9bc}

linear · 2026-02-22T03:48:36Z

FIRE-10 Wire `exitWithError()` and `output()` across all CLI commands

galligan · 2026-02-22T03:48:45Z

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.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

…dlers - approve, reject, comment: fully delegated to core handlers (approveHandler, rejectHandler, commentHandler), removing createContext/client wiring - reply, edit: replaced console.error + process.exit(1) with exitWithError() (handler delegation deferred due to feature gaps in core handlers) - status, doctor: also migrated to exitWithError() for consistency - Added missing handler re-exports to packages/core/src/index.ts 🤘🏻 In-collaboration-with: [Claude Code](https://claude.com/claude-code)

greptile-apps

_{8 files reviewed, 4 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-22T04:10:28Z

    if (ctx.outputJson) {
      await outputStructured(result, "jsonl");
-    } else {
-      console.error(`Failed to delete comment ${displayId}: ${result.error}`);
+      process.exit(1);
    }
-    process.exit(1);
+    exitWithError(
+      new Error(`Failed to delete comment ${displayId}: ${result.error}`)
+    );


process.exit(1) called before exitWithError(), making the error handler unreachable

Suggested change

if (ctx.outputJson) {

await outputStructured(result, "jsonl");

} else {

console.error(`Failed to delete comment ${displayId}: ${result.error}`);

process.exit(1);

}

process.exit(1);

exitWithError(

new Error(`Failed to delete comment ${displayId}: ${result.error}`)

);

if (ctx.outputJson) {

await outputStructured(result, "jsonl");

}

exitWithError(

new Error(`Failed to delete comment ${displayId}: ${result.error}`)

);

Prompt To Fix With AI

This is a comment left during a code review. Path: apps/cli/src/commands/edit.ts Line: 552-558 Comment: `process.exit(1)` called before `exitWithError()`, making the error handler unreachable ```suggestion if (ctx.outputJson) { await outputStructured(result, "jsonl"); } exitWithError( new Error(`Failed to delete comment ${displayId}: ${result.error}`) ); ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-22T04:10:28Z

    if (ctx.outputJson) {
      await outputStructured(result, "jsonl");
-    } else {
-      console.error(`Failed to delete comment ${displayId}: ${result.error}`);
+      process.exit(1);
    }
-    process.exit(1);
+    exitWithError(
+      new Error(`Failed to delete comment ${displayId}: ${result.error}`)
+    );


process.exit(1) called before exitWithError(), making the error handler unreachable

Suggested change

if (ctx.outputJson) {

await outputStructured(result, "jsonl");

} else {

console.error(`Failed to delete comment ${displayId}: ${result.error}`);

process.exit(1);

}

process.exit(1);

exitWithError(

new Error(`Failed to delete comment ${displayId}: ${result.error}`)

);

if (ctx.outputJson) {

await outputStructured(result, "jsonl");

}

exitWithError(

new Error(`Failed to delete comment ${displayId}: ${result.error}`)

);

Prompt To Fix With AI

This is a comment left during a code review. Path: apps/cli/src/commands/edit.ts Line: 591-597 Comment: `process.exit(1)` called before `exitWithError()`, making the error handler unreachable ```suggestion if (ctx.outputJson) { await outputStructured(result, "jsonl"); } exitWithError( new Error(`Failed to delete comment ${displayId}: ${result.error}`) ); ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-22T04:10:30Z

    if (ctx.outputJson) {
      await outputStructured(result, "jsonl");
-    } else {
-      console.error(`Failed to update comment ${displayId}: ${result.error}`);
+      process.exit(1);
    }
-    process.exit(1);
+    exitWithError(
+      new Error(`Failed to update comment ${displayId}: ${result.error}`)
+    );


process.exit(1) called before exitWithError(), making the error handler unreachable

Suggested change

if (ctx.outputJson) {

await outputStructured(result, "jsonl");

} else {

console.error(`Failed to update comment ${displayId}: ${result.error}`);

process.exit(1);

}

process.exit(1);

exitWithError(

new Error(`Failed to update comment ${displayId}: ${result.error}`)

);

if (ctx.outputJson) {

await outputStructured(result, "jsonl");

}

exitWithError(

new Error(`Failed to update comment ${displayId}: ${result.error}`)

);

Prompt To Fix With AI

This is a comment left during a code review. Path: apps/cli/src/commands/edit.ts Line: 617-623 Comment: `process.exit(1)` called before `exitWithError()`, making the error handler unreachable ```suggestion if (ctx.outputJson) { await outputStructured(result, "jsonl"); } exitWithError( new Error(`Failed to update comment ${displayId}: ${result.error}`) ); ``` How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-22T04:10:31Z

    if (ctx.outputJson) {
      await outputStructured(result, "jsonl");
-    } else {
-      console.error(`Failed to update comment ${displayId}: ${result.error}`);
+      process.exit(1);
    }
-    process.exit(1);
+    exitWithError(
+      new Error(`Failed to update comment ${displayId}: ${result.error}`)
+    );


process.exit(1) called before exitWithError(), making the error handler unreachable

Suggested change

if (ctx.outputJson) {

await outputStructured(result, "jsonl");

} else {

console.error(`Failed to update comment ${displayId}: ${result.error}`);

process.exit(1);

}

process.exit(1);

exitWithError(

new Error(`Failed to update comment ${displayId}: ${result.error}`)

);

if (ctx.outputJson) {

await outputStructured(result, "jsonl");

}

exitWithError(

new Error(`Failed to update comment ${displayId}: ${result.error}`)

);

Prompt To Fix With AI

This is a comment left during a code review. Path: apps/cli/src/commands/edit.ts Line: 656-662 Comment: `process.exit(1)` called before `exitWithError()`, making the error handler unreachable ```suggestion if (ctx.outputJson) { await outputStructured(result, "jsonl"); } exitWithError( new Error(`Failed to update comment ${displayId}: ${result.error}`) ); ``` How can I resolve this? If you propose a fix, please make it concise.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 056e9bcc2e

ℹ️ 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".

chatgpt-codex-connector · 2026-02-22T04:12:24Z

+    const result = await commentHandler(
+      { pr, body, repo },
+      { config, db, logger: silentLogger }
    );


Preserve transport errors when delegating comment creation

Routing comment through commentHandler changes failure semantics: commentHandler converts any fetchPullRequestId failure into NotFoundError (in packages/core/src/handlers/comment.ts), so auth failures or transient GitHub/network errors are now reported as “PR not found.” This breaks the new category-based exit behavior for real-world cases like expired tokens or API outages and makes retries/diagnosis incorrect.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-02-22T04:12:24Z

  try {
-    const ctx = await createContext(options);
+    const config = await loadConfig();
+    const db = getDatabase();


Remove mandatory SQLite initialization from write-only commands

Initializing db here introduces a new hard dependency on local cache storage for a command that only performs a GitHub mutation. getDatabase() creates directories/files under the cache path (mkdirSync + openDatabase in packages/core/src/cache.ts), so fw comment can now fail before hitting GitHub in read-only/containerized environments where it previously worked.

Useful? React with 👍 / 👎.

This was referenced Feb 22, 2026

feat(mcp): adopt @outfitter/mcp with createMcpServer() and defineTool() #171

Open

docs: update architecture docs for Outfitter stack adoption #172

Open

galligan force-pushed the fire-9/cli-framework branch from e19b65d to abcf1dd Compare February 22, 2026 03:51

galligan force-pushed the fire-10/cli-output branch from 9244256 to 056e9bc Compare February 22, 2026 03:51

galligan changed the title ~~refactor(cli): wire approve, reject, comment, reply, edit to core handlers~~ refactor(cli): wire exitWithError() across key commands Feb 22, 2026

galligan marked this pull request as ready for review February 22, 2026 04:08

greptile-apps Bot reviewed Feb 22, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Feb 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(cli): wire exitWithError() across key commands#170

refactor(cli): wire exitWithError() across key commands#170
galligan wants to merge 1 commit into
fire-9/cli-frameworkfrom
fire-10/cli-output

galligan commented Feb 22, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

linear Bot commented Feb 22, 2026

Uh oh!

galligan commented Feb 22, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot left a comment

Uh oh!

greptile-apps Bot Feb 22, 2026

Uh oh!

greptile-apps Bot Feb 22, 2026

Uh oh!

greptile-apps Bot Feb 22, 2026

Uh oh!

greptile-apps Bot Feb 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Feb 22, 2026

Uh oh!

chatgpt-codex-connector Bot Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

galligan commented Feb 22, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Uh oh!

linear Bot commented Feb 22, 2026

Uh oh!

galligan commented Feb 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

galligan commented Feb 22, 2026 •

edited by greptile-apps Bot

Loading

galligan commented Feb 22, 2026 •

edited

Loading