Skip to content

Conversation

@sjawhar
Copy link

@sjawhar sjawhar commented Feb 10, 2026

Summary

Guard against undefined output.output in all 14 tool.execute.after hooks and injectors that access the field.

MCP tool results (e.g. Slack, Toggl) don't populate output.output as a string, causing:

  • TypeError crashes in hooks that call .toLowerCase() or .startsWith() on undefined
  • Silent "undefined" string corruption in hooks that use += (producing "undefined\n\nSome reminder")

Root Cause

The tool.execute.after hook fires for all tool calls, including MCP tools. The hook type signature declares output: { output: string }, but MCP tool results don't always populate this field. Many hooks access output.output without null-checking first.

Two existing hooks already guarded against this:

  • empty-task-response-detector uses output.output?.trim() ?? ""
  • tool-output-truncator used typeof output.output !== 'string'

The rest did not, and the codebase was inconsistent.

Fix

Add output.output == null early-return guard to every tool.execute.after handler that accesses output.output. This:

  • Uses loose equality (== null) to catch both null and undefined
  • Allows empty strings through (which are valid tool outputs)
  • Is applied consistently across all hooks, including those behind tool filters (defense in depth)
  • Normalizes the previous typeof !== 'string' check in tool-output-truncator to match

Hooks fixed

Hook Failure mode Had tool filter?
comment-checker Crash: .toLowerCase() on undefined No — crashed before filter
edit-error-recovery Crash: .toLowerCase() on undefined Yes (edit only)
task-resume-info Crash: .startsWith() on undefined Yes (task/call_omo_agent)
delegate-task-retry Passed undefined to function Yes (task only)
context-window-monitor Corrupt: undefined += No
task-reminder Corrupt: undefined += No
claude-code-hooks handler Corrupt: template literal No
category-skill-reminder Corrupt: undefined += Yes
agent-usage-reminder Corrupt: undefined += Yes
interactive-bash-session Corrupt: undefined += Yes (interactive_bash)
directory-readme-injector Corrupt: undefined += Yes (read/glob/grep)
directory-agents-injector Corrupt: undefined += Yes (read/glob/grep)
rules-injector Corrupt: undefined += Yes (read/glob/grep)
tool-output-truncator Already guarded (normalized) Yes

Testing

  • Built locally, loaded via file:// plugin path in OpenCode config
  • Verified Slack MCP conversations_add_message and channels_list calls succeed after fix (previously all Slack MCP calls returned TypeError)
  • All existing tool functionality (edit, bash, read, grep, etc.) unaffected — the guard only skips when output.output is nullish

@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2026

All contributors have signed the CLA. Thank you! ✅
Posted by the CLA Assistant Lite bot.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Auto-approved: Simple defensive type check additions. Both changes add typeof output.output !== \"string\" guards before calling .toLowerCase(), preventing runtime errors on non-string outputs. No logic changes,

@sjawhar sjawhar force-pushed the fix/guard-undefined-tool-output branch from 761a42e to 47df964 Compare February 10, 2026 17:05
….after hooks

MCP tool results (e.g. Slack, Toggl) don't populate output.output as a
string, causing TypeError crashes in hooks that call .toLowerCase() or
.startsWith(), and silent "undefined" string corruption in hooks that
use +=.

Add `output.output == null` guard to all 14 tool.execute.after hooks
and injectors that access output.output. This is consistent with the
existing guard style in empty-task-response-detector (which uses ?.)
and normalizes the previous typeof check in tool-output-truncator.

Hooks fixed:
- comment-checker (crash: .toLowerCase on undefined)
- edit-error-recovery (crash, behind tool filter)
- task-resume-info (crash: .startsWith on undefined, behind tool filter)
- delegate-task-retry (passed to function, behind tool filter)
- context-window-monitor (corrupt: += on undefined)
- task-reminder (corrupt: += on undefined)
- claude-code-hooks handler (corrupt: template literal with undefined)
- category-skill-reminder (corrupt, behind tool filter)
- agent-usage-reminder (corrupt, behind tool filter)
- interactive-bash-session (corrupt, behind tool filter)
- directory-readme-injector (corrupt, behind tool filter)
- directory-agents-injector (corrupt, behind tool filter)
- rules-injector (corrupt, behind tool filter)
- tool-output-truncator (normalize typeof to == null)
@sjawhar sjawhar force-pushed the fix/guard-undefined-tool-output branch from 47df964 to 76e3a51 Compare February 10, 2026 17:10
@sjawhar
Copy link
Author

sjawhar commented Feb 10, 2026

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Feb 10, 2026
marlon-costa-dc added a commit to marlon-costa-dc/oh-my-opencode that referenced this pull request Feb 10, 2026
@sjawhar sjawhar closed this Feb 12, 2026
@sjawhar sjawhar deleted the fix/guard-undefined-tool-output branch February 12, 2026 13:47
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