Skip to content

fix: add error handling to 8 tools with unguarded async handlers - #221

Open
TerminalGravity wants to merge 1 commit into
mainfrom
fix/error-handling-8-tools
Open

fix: add error handling to 8 tools with unguarded async handlers#221
TerminalGravity wants to merge 1 commit into
mainfrom
fix/error-handling-8-tools

Conversation

@TerminalGravity

Copy link
Copy Markdown
Collaborator

8 tools had no try/catch around their async handlers. If git wasn't available, state files were corrupt, or directories were missing, they'd throw unhandled errors and crash the MCP server.

Now they return user-friendly error messages (e.g. ❌ checkpoint failed: ...) instead of crashing.

Tools fixed: audit_workspace, check_patterns, checkpoint, search_contracts, sequence_tasks, session_health, sharpen_followup, what_changed

Tools that previously had no try/catch and would crash on errors
(e.g., git not available, corrupt state files, missing dirs) now
return user-friendly error messages instead of throwing:

- audit_workspace
- check_patterns
- checkpoint
- search_contracts
- sequence_tasks
- session_health
- sharpen_followup
- what_changed

@TerminalGravity TerminalGravity left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good defensive addition. The try/catch wrappers ensure MCP tools return error content instead of crashing the server process.

One concern: this PR still uses the broken run() calls (e.g., what-changed.ts line 13 still has run(\git diff ...`)`). This should be rebased on top of #220 after it merges, otherwise you're wrapping error handlers around calls that silently fail anyway.

Also, the pattern could be DRY'd with a wrapper:

function safeTool(fn: () => Promise<ToolResult>): Promise<ToolResult> {
  try { return await fn(); }
  catch (err) { return { content: [{ type: 'text', text: \`❌ \${err}\` }] }; }
}

Not blocking, but worth considering to avoid the indent creep across 8 files.

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